views:

819

answers:

6

When implementing a factory or simple factory, what would go against using a Type instead of an Enum to specify the class to instantiate?

For example

public class SimpleFactory
{
 public static ITest Create(Type type)
 {
  if (type == typeof(ConcreteTest1))
   return new ConcreteTest1();
  if (type == typeof(ConcreteTest2))
   return new ConcreteTest2();

  throw new Exception("Invalid type");
 }
}
+14  A: 

Using an enum is more restrictive, which means that it is less likely that the user will try to use your factory with an unsupported type.

I find that it's good to do everything possible when defining an API to discourage usage patterns that will cause exceptions to be thrown. Allowing "Type" in this case opens up millions of ways to call your function that will result in:

throw new Exception("Invalid type");

Using an enum would eliminate this. The only way an enum would throw would be if the user did something noticably wrong.

Reed Copsey
I definitely agree! Compile-time constraints are much easier to enforce and easier to use.
ph0enix
+2  A: 

if you want a fool proof factory you must create one concrete factory for each concrete type. This class doesn't follow open-closed principle: each time you got a new concrete type you've to re-edit this class.

IMHO a better approach is using inheritance, one concrete factory class for each concrete type.

dfa
A: 

If you want to create by type, you could just use Activator.CreateInstance(Type t). Wrap it in a template method to limit it to your interface, something like Create<T> where T:ITest.

Robert
+1  A: 

I would prefer to use a generic constraint, for the reason that having an enum just to specify what kind of object you want seems redundant to me, and with using a type as you've described you violate the Open/Closed principle. What I would do differently from what you have done there is constrain your type so that only allowable types can be passed in.

I'll give an example in c# using generics.

public class SimpleFactory
{
 public static ITest Create<T>()
    where T: ITest, new()
 {
    return new T();
 }
}

Then you would implement IConcreteTest with both ConcreteTest1 and ConcreteTest2 and you could use your factory like this:

ConcreteTest1 test1 = SimpleFactory.Create<ConcreteTest1>();
Joseph
How does passing the type to a generic method fix the problem? Doesn't that still require some advanced knowledge of T by the caller?
ph0enix
Yes, but in his example he already knows the type, he's passing it into the method. The only difference is that I'm using it as a generic instead so you can do something like SimpleFactory.Create<ConcreteTest1>();
Joseph
He hasn't given the implementation of typeof, so we can't be sure he has advance knowledge of the types and can pass in to the call.
Rohit
@Rohit implementation of typeof? typeof is a keyword (http://msdn.microsoft.com/en-us/library/58918ffs(VS.71).aspx) and he does have advance knowledge, otherwise he wouldn't have passed it in as a parameter
Joseph
and for sure you'll have advanced knowledge if you're asking about passing in an ENUM
ScottCate
A: 

I think the biggest concern that I would have is that the purpose of the factory is to allow client code to create a derived instance of an object without knowing the details of the type being created (more specifically, the details of how to create the instance, but if done correctly, the caller should not need to know any of the finer details beyond what is provided by the base class).

Using type information extracted from the derived type still requires the caller to have some intimate knowledge about which type he wants to instantiate, which makes it difficult to update and maintain. By substituting an Enum type (or string, int, etc.), you can update the factory without having to update the calling code to be aware of the new derived types.

I suppose one might argue that the type name could be read in as a string from a config file, database, etc., and the type information determined using Reflections (in .NET) or RTTI (in C++), but I think this is a better case for simply using the type string as your identifier since it will effectively serve the same purpose.

ph0enix
+3  A: 

Factories are only useful if they perform configuration or initialization on your objects to put them in a valid state. I wouldn't bother with a factory if all it does is new up and return objects.

I would create a factory for each class hierarchy. For example:

public abstract class Vehicle {}
public class Car : Vehicle {}
public class Truck : Vehicle {}

public class VehicleFactory
{
    public Vehicle CreateVehicle<T>() where T : Vehicle
    {
        // Get type of T and delegate creation to private methods
    }
}
Jamie Ide