Is this a good factory method implementation?

Question

I'm working on a module that requires a strictly decoupled interface. Specifically, after instantiating the root object (a datasource), the user's only supposed to interact with the object model via interfaces. I have actual factory objects (I'm calling them providers) to supply instances that implement these interfaces, but that left the clumsiness of getting the providers. To do so, I've supplied a couple methods on the datasource:

public class MyDataSource
{
    private Dictionary<Type, Type> providerInterfaceMapping = new Dictionary<Type, Type>()
    {
        { typeof(IFooProvider), typeof(FooProvider) },
        { typeof(IBarProvider), typeof(BarProvider) },
        // And so forth
    };

    public TProviderInterface GetProvider<TProviderInterface>()
    {
        try
        {
            Type impl = providerInterfaceMapping[typeof(TProviderInterface)];
            var inst = Activator.CreateInstance(impl);

            return (TProviderInterface)inst;
        }
        catch(KeyNotFoundException ex)
        {
            throw new NotSupportedException("The requested interface could not be provided.", ex);
        }
    }
}

I've modified some details on the fly to simplify (e.g., this code snippet doesn't include the parameters passed to the implementation instance that's created). Is this a good general approach for implementation of a factory method in C#?

Answer 1

You should rather take a step back and ask whether using a factory method at all is a good idea? In my opinion, it is not.

There are more than one issue with factory methods, and your example illustrates several:

• You need to have a hard reference to the implementation (FooProvider in addition to IFooProvider), which is exactly the situation you are trying to avoid in the first place. Even if the rest of your code only consumes IFooProvider, your library is still tightly coupled to FooProvider. Some other developer may come by and start using FooProvider directly if he/she isn't aware of your factory method.
• You only support implementations that have default constructors, since you are using Activator.CreateInstance. This prevents you from using nested dependencies.

Instead of trying to manually control dependencies, I would recommend that you take a look at Dependency Injection (DI). Whenever your code needs an IFooProvider, supply it with Constructor Injection.