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.

Answer 2

Technically this is fine, however most times when I see a factory it usually returns the same type interface, for instance something like IProvider rather than IFooProvider or IBarProvider which to me doesn't make sense. If you are going to have FooProvider and BarProvider then why have different interfaces for them. I would use one interface IProvider and have FooProvider and BarProvider implement that.

Answer 3

Regardless of the rightness or wrongness of using the factory method (as that is not what you asked about!), your implementation looks fine to me.

Something that may work for you better than hardcoding the type mapping is putting that info in a configuration file and loading it in your app.

Answer 4

Don't reinvent your own implementation of dependency injection, use an existing library like Spring.NET or the Microsoft Unity application block.

Injecting dependencies is a common programming problem that you shouldn't have to solve yourself. There are some nice lightweight libraries out there (I mentioned a couple above) that do the job well. They support both declarative and imperative models of defining dependencies and are quite good at what they do.

Answer 5

For what it is worth I use this pattern all the time and have abstracted some of this sort of logic into a reusable assembly. It uses reflection, generics and attributes to locate and bind the concrete types at runtime. http://www.codeproject.com/KB/architecture/RuntimeTypeLoader.aspx

This helps to address Mark's concern because implementation types are not hardcoded, and further the implementation types are determined by the installation, not in project assembly references.