Reputation: 31
I have some code in an application that I am not really thrilled about right now. I created a few classes like so:
class Base
{
// base properties ...
}
class DerivedA : Base
{
}
class DerivedB : Base
{
}
I have a method in my application that needs to create one of these objects (with more to come in the future) based on a string property that is stored in the database. Each one of these objects gets its data from slightly different places, but the way I'm doing it right now is just a big if block and it doesn't seem very maintainable:
class BaseCreator
{
Base Create(string name)
{
if (name == "DerivedA" )
return CreateDerivedA();
else if(name == "DerivedB")
return CreateDerivedB();
}
}
What are some ways I can refactor this code to be more maintainable and make it easier to add new types in the future? I am using dependency injection (Ninject) in my application if that makes any difference.
Upvotes: 3
Views: 951
Reputation: 672
There isn't a general answer to this question. Abstract Factory may be correct, but it all depends on what the difference is between these implementations and how you use them.
It could well be that you should be using Template, Strategy, State or any other similar pattern. Look into them, and definitely Abstract Factory, and decide on a pattern that suits your specific scenario.
Upvotes: 0
Reputation: 19613
If you really must use strings, you can use reflection:
object GetInstance(string typeName)
{
Type.GetType(typeName).GetConstructor(Type.EmptyTypes).Invoke(new object[0]);
}
You could also use a dictionary:
IDictionary<string, Func<object>> TypeMap = new Dictionary<string, Func<object>>()
{
{ "TypeA", () => new TypeA() },
{ "TypeB", () => new TypeB() },
{ "TypeC", () => new TypeC() },
};
object GetInstance(string typeName)
{
return TypeMap[typeName]();
}
For others landing on this page, consider using generics, if you don't have to use strings:
T CreateInstance<T>()
where T : new()
{
return new T();
}
Upvotes: 1
Reputation: 233135
The general case can be solved by a bit of composition and the use of the Specification pattern:
public class Base
{
public abstract bool IsSatisfiedBy(string name);
// base properties ...
}
public class DerivedA : Base
{
public override bool IsSatisfiedBy(string name)
{
return name == "DerivedA";
}
}
public class DerivedB : Base
{
public override bool IsSatisfiedBy(string name)
{
return name == "DerivedB";
}
}
public class BaseCreator
{
private readonly IEnumerable<Base> candidates;
public BaseCreator(IEnumerable<Base> candidates)
{
this.candidates = candidates;
}
public Base Create(string name)
{
return this.candidates.First(c => c.IsSatisfiedBy(name));
}
}
Upvotes: 1
Reputation: 126804
I will note that your if/else
or switch
structure is not a bad thing. The bad thing is when you have the same if/else
or switch
expressed multiple times.
When you have nicely decoupled your code and you are programming to the interface or abstract base rather than the concrete, know that somewhere in your application, something knows how to create the particular concrete instance that you need. This can be code, it can be configuration, it can be some container, etc. But that something has to exist. The idea is to have that something existing once.
Your approach is fine as long as this is the only method where it exists. This class' reason to exist is that it creates the concrete instances that fulfill some interface. Its reason to change is that some other concrete implementation has been added (or removed).
Upvotes: 1
Reputation: 6612
I think you should use the abstract factory pattern which solves this problem. It provides an interface for creating families of related or dependent objects without specifying their concrete classes. http://www.dofactory.com/Patterns/PatternAbstract.aspx
Or just a factory pattern http://www.dotnetperls.com/factory
Upvotes: 1
Reputation: 4032
Inheritance trees are always difficult to maintain once they grow. If you know up front that the tree will be large---seriously consider using composition instead of inheritance. Especially if you're already using a DI framework, interfaces are the way to go.
Upvotes: 1