What creational pattern I should use?

Question

My program have two classes; both derive from same base class.

class A : MyBase
{
    internal A(InitVal initVal)
}

class B : MyBase
{
    internal B(InitVal initVal)
}

InitVal is another class which is injected through constructor. This class is for internal usage. Due to internal constructor, user cannot create instance of class A and B directly. Instead, I created method which creates these objects.

class Initiator
{
    InitVal initVal;

    public T CreateObject<T>(ObjectInstance objectInstance) where T : MyBase
    {
        MyBase myBase = null;
        switch(objectInstance)
        {
            case ObjectInstance.A:
                myBase = new A(initVal);
                break;
            case ObjectInstance.B:
                myBase = new B(initVal);
                break;
        }
        return (T)myBase;
    }
    ...
}

ObjectInstance is enum in above code.

This works without problem but I am sure you have never seen such ugly code earlier.

Please suggest creational pattern I should use. I want to remove ObjectInstance enum without changing functionality. It will cleanup much.

I tried Creational Patterns mentioned on dotfactory. Factory Method and Abstract Factory does not look proper in this case.

My code even though look ugly, it is very simple to read and understand. I tried implementing patterns mentioned above which increases code complexity. So this is also my criteria while choosing answer.

I cannot change anything in code except Initiator class. All other classes are not accessible to me for edit.

Edit 1: Why above code is ugly in my view

1) While calling CreateObject method, user have to specify type of the object twice.

A a = initiator.CreateObject<A>(ObjectInstance.A);

First for T generic value and second to enum value. I want to avoid this.

2) As user has to specify type of object twice, there are chances of mistake.

A a = initiator.CreateObject<A>(ObjectInstance.B);

In above code, enum value and generic value are different. This is not allowed and will be a problem. With my code, I cannot avoid this.

That is why; I am looking for pattern that suits my case without increasing complexity.

If I remove necessity of enum somehow, code will be lot better. If I can change signature of CreateObject to following, it will be lot better.

public T CreateObject<T>() where T : MyBase

But, I am not sure how I will implement this method to create proper instances.

Answer 1

It doesn't look to me like you are getting any advantage from trying to make this generic. You needs to know the concrete type of the returned value at the call site.

Therefore why not keep things simple and just do this?

public class Initiator
{
    InitVal initVal;

    public A CreateA()
    {
        return new A(initVal);
    }

    public B CreateB()
    {
        return new B(initVal);
    }
}

Answer 2

One obvious problem with your code is the enum, which is unnecessary, because typeof(T) already gives you the appropriate type:

class Initiator
{
    readonly Dictionary<Type, Func<MyBase>> _dict = new Dictionary<Type, Func<MyBase>>();

    internal Initiator(InitVal initVal)
    {
        // initialize your "service locator".
        // it's cool that different types can have different constructors,
        // and people who call CreateObject don't need to know this.
        _dict[typeof(A)] = (Func<MyBase>)(() => new A(initVal));
        _dict[typeof(B)] = (Func<MyBase>)(() => new B(initVal, someOtherStuff));
    }

    public T CreateObject<T>() where T : MyBase
    {
        var ctor = _dict[typeof(T)];
        return (T)ctor();
    }
}

Alternatively, if you don't know the type, you can pass the enum, but then the return type should be an interface/base class (preferably interface):

// this is more likely, you probably don't need a generic method
public IMyBase CreateObject(ObjectInstance objectInstance)
{
    // the dictionary would map enum value to Func<IMyBase>, of course
    var ctor = _dict[objectInstance];
    return ctor();
}

And now you have a simple "poor man's" DI class called Initiator, so I wonder if your DI framework (the one which injected InitVal) can also inject A and B instances. Which is probably true, since DI purists will tell you there is no place for factories and the new keyword in your code.

Btw, ObjectInstance is a really, really bad name for an enum.

Answer 3

I did it in following way:

class A : IMyType
{
    internal A(InitVal initVal)
}

class B : IMyType
{
    internal B(InitVal initVal)
}

class Initiator
{
    InitVal initVal = .....;

    public T CreateObject<T>() where T : IMyType
    {
        IMyType myType = null;
        if(typeof(T) == typeof(A))
            myType = new A(initVal);
        else if(typeof(T) == typeof(B))
            myType = new B(initVal);
        else
            throw new MyException("Type is not configured.");
        return (T)myType;
    }
    ...
}

This resolves the problems I mentioned in my question. But, it creates new problem. This violates open-close principle of SOLID. Last else block handles the manual mistake if any. Anyway, it just works for my specific case; not recommended generally.

Answer 4

As you specified the method as generic one, I expect you might actually know the type you want to get already during the compilation time.. so I'd go for something like this:

class Initiator
{ 
    public T CreateObject<T>(ObjectInstance objectInstance) where T : MyBase, new()
    {
        T newInstance = new T();
        newInstance.Value = initVal;

        return newInstance;
    }
...
}

now you can call it as:

A myAInstance = initiator.CreateObject<A>();
MyBase myAInstance = initiator.CreateObject<A>();   //this also works

To make it work you need to specify an internal parameterless constructor in your classes and specify interface for the Value property or whatever you would set now in your current constructor.

class MyBase{
    InitVal Value { get; set;}       //this allows construction of the object with parameterless constructor
    ...
}

This is not only cleaner and shorter, but also less error prone, as you dont need to edit both enum and method body every time new type is added. It gives less flexibility for child-type specific logic, though.

NOTE: If you really want to have constructor with parameters as you have now you still can go for this approach but you'd need to use reflection (check Activator) or lambdas.

Of course this makes only sense if you can decide on the type during compilation time or you if you just want to delegate this decition to a 3rd party library, eg:

switch(chosenType){
case ObjectInstance.A:
    instance = initiator.CreateObject<A>();
    ...

Otherwise, simply leave it as it is, its a FactoryMethod pattern more or less and it does the job. Just that the generic parameter in it... seems to be quite useless then. I would remove it and change return type to MyBase, as user won't be able to specify T anyway.

One last option is to simply create a separate method for each type, this is clean, flexible, gives a lot of options for customization, but sucks if you need to repeat a lot of shared logic and you need to add a new one for each next type. Simply:

A CreateObjectA(InitVal initValue){
     return new A(initValue);
}
B CreateObjectB(InitVal initValue){ ...