A static factory method on a generic or a non-generic class?

Question

I have a generic class, let's call it MyClass<T>, which will need to have a factory method so to abstract constructor details away from client code.

Which of the two options is correct? (example instantiation code included)

Static non-generic factory method on the original generic MyClass<T>:
```
MyClass<SomeType> instance = MyClass<SomeType>.CreateNew();
```
Static generic factory method on a dedicated, non-generic static MyClass implementation:
```
MyClass<SomeType> instance = MyClass.CreateNew<SomeType>();
```

I think this question is a good candidate for [What's your most controversial programming opinion?](http://stackoverflow.com/questions/406760/whats-your-most-controversial-programming-opinion) but only for C# language — Ilya Ivanov, May 21 '13 at 09:34

Zoran Horvat · Accepted Answer · 2013-05-21T10:31:32.223

On the first sight, looks like proper answer to your question is #1. This is because your class is MyClass<T> and hence factory should also be T-specific. But there is more into it than this simple answer.

Before proceeding, I would add the third possibility: non-static factory class. Object which relies on a factory would have a public property through which it receives a factory object. Getter of the property would instantiate default factory if no other instance has been assigned. This allows dependency injection later, and also helps write unit tests that rely on their own fake factory. Solution would look something like this (ignore generics for a moment):

public class ISomeFactory { ... }

public class DefaultSomeFactory: ISomeFactory { ... }

public class ClientClass
{
    public ISomeFactory FactoryOfSome  // Right place for dependency injection
    {
        get
        {
            if (_fact == null)
                _fact = new DefaultFactory();
            return _fact;
        }
        set { _fact = value; }
    }
    private ISomeFactory _fact;
}

Now as I said, your class goes with generic parameter MyClass<T> and then factory should go with generic parameter as well: Factory<T>. This solution is better than generic method on general factory, simply because creating an instance may be T-specific. Solution with generic factory allows you this:

public class SpecialSomeFactory: DefaultSomeFactory<string> { ... }

In this way, you can override behavior of existing factory class and have an alternative way to generate instances that are specialized for strings. This is important because dealing with strings is often much different than dealing with primitive types like int or double. Having an opportunity to specialize the factory may be beneficial. But now you see why it might be a bad idea to have a static factory - statics cannot be extended.

Many experienced developer with test unit experience will know that both option is bad :). IMHO, it is better to do constructor injection than lazy get since it is easier to mock and prevent `magic dependency`. +1 btw. — Fendy, May 21 '13 at 10:19
@Fendy - You're right about constructors. General rule that I follow is to use constructor injection for mandatory parameters, and property injection for optional ones. Lazy get is misused too often. I use it only as a pattern of property overriding. In example above, property has default implementation which is generally not supposed to be changed ever. Only once in a while (e.g. in unit tests) you will change it with a different implementation. Trick is in this question: Is changing a property value going to break the code? YES means you hardcode it; NO means IoC container is the right way. — Zoran Horvat, May 21 '13 at 10:38
If `changing a property value going to break the code` means that you need to fix the implementation, not hardcode the default value. What I consider cons in `Lazy` property is, if it need to be changed (the DefaultFactory), then you must do in every place. Don't worry thoug, it may not applied in this (your) case, but can happen in other case. Code abusive is easy though — Fendy, May 21 '13 at 10:45

score 1 · Answer 2 · answered May 21 '13 at 10:48

Both examples are essentially variations on the service locator pattern. Controversially, this is often considered an anti-pattern as the dependencies of a class that uses it are not visible to its consumers.

A way to make your dependencies more explicit would be to adopt a solution similar to the first example, but make CreateNew() an instance method, as opposed to a static one.

A static factory method on a generic or a non-generic class?

2 Answers2