I have two classes like below:
public async A GernerateStuff(int expireDays = 15)
{
using var randomNumberGenerator = RandomNumberGenerator.Create();
var randomBytes = new byte[64];
var now = DateTime.UtcNow;
randomNumberGenerator.GetBytes(randomBytes);
return new A
{
Stuff = Convert.ToBase64String(randomBytes),
Created = now,
Expires = now.AddDays(expireDays)
};
}
public async B GernerateStuff(int expireDays = 10)
{
using var randomNumberGenerator = RandomNumberGenerator.Create();
var randomBytes = new byte[64];
var now = DateTime.UtcNow;
randomNumberGenerator.GetBytes(randomBytes);
return new B
{
Stuff = Convert.ToBase64String(randomBytes),
Created = now,
Expires = now.AddDays(expireDays)
};
}
public class A
{
public string Stuff{ get; set; }
public DateTime Created { get; set; }
public DateTime Expires { get; set; }
}
public class B
{
public string Stuff{ get; set; }
public DateTime Created { get; set; }
public DateTime Expires { get; set; }
}
The constraint is: I can not create just one class instead of two separate classes A and B as they have significant differences in usage.
Now, my question is: how can I clean this code up having both classes A and B but a single method for GernerateStuff
?
I can create an interface like this:
public class A : IInterface
{
}
public class B : IInterface
{
}
public interface IInterface
{
public string Stuff{ get; set; }
public DateTime Created { get; set; }
public DateTime Expires { get; set; }
}
Then, the problem is how public async IInterface GernerateStuff(int expireDays = 15)
signature would handle both class A and B?
CodePudding user response:
There are a couple of ways tot achieve this. One has already been mentioned in other answeres, which is about using generics. However this assumes your types A
and B
do even have anything in common that you could use as common generic constraint - e.g a common base-interface. Then you could do this:
public async T GernerateStuff<T>(int expireDays = 15) where T: new(), MyInterface
{
using var randomNumberGenerator = RandomNumberGenerator.Create();
var randomBytes = new byte[64];
var now = DateTime.UtcNow;
randomNumberGenerator.GetBytes(randomBytes);
return new T
{
Stuff = Convert.ToBase64String(randomBytes),
Created = now,
Expires = now.AddDays(expireDays)
};
}
You could also create a factory that creates the instances of A
or B
depending on some condition - e.g. some configuration. However you'd also need a common base-interface for that:
public async MyInterface GernerateStuff(int expireDays = 15)
{
using var randomNumberGenerator = RandomNumberGenerator.Create();
var randomBytes = new byte[64];
var now = DateTime.UtcNow;
randomNumberGenerator.GetBytes(randomBytes);
return CreateTheThing(Convert.ToBase64String(randomBytes), now, now.AddDays(expireDays));
}
MyInterface CreateTheThing(string stuff, DateTime created, DateTime expires)
{
if(...)
return new A { ... }
else if(...)
return new B { ... }
return new C { ... }
}
This solution has the advantage, that you don't need to change the client-logic when you add a new type to the factory. All you need to change is the factory itself by introducing new C { ... }
. Furthermor a client cannot provide any types that actually don't work, as they don't provide any type-information at all.
By the way your method does not await
anything, so there's no reason to make it async
.
CodePudding user response:
Create a static factory method and inject it as a parameter:
public static A CreateA(byte[] stuff, DateTime now, DateTime expires)
=> new A{ Stuff = stuff, Created = now, Expires = expires }
...
public static T GernerateStuff<T>(Func<byte[], DateTime, DateTime, T> ctor, int expireDays = 15)
{
using var randomNumberGenerator = RandomNumberGenerator.Create();
var randomBytes = new byte[64];
var now = DateTime.UtcNow;
randomNumberGenerator.GetBytes(randomBytes);
return ctor(
Convert.ToBase64String(randomBytes),
now,
now.AddDays(expireDays)
;
}
...
var a = GernerateStuff(CreateA);
But if class A and B are identical, why not use the same class? This is a definite code smell, and you should probably think about how you model your problem some more. Also, async
does not serve any point without an await, so get rid of it.