2

I am currently stuck at trying to write a factory class that doesn't rely on service location.

The only other alternative I can think of is to use constructor injection to inject all possible instances, but that may lead to surprises as classes are passed via reference. It is also possibly going to be costly and messy once the number of possible providers grow.

The providers themselves are full complex classes that have their own dependencies so manual construction is out of the picture.

Updated service location example:

    public class ProviderFactory : IProviderFactory
    {
        private readonly IProviderConfigurationService _providerConfigurationService;

        public enum SearchType
        {
            Foo,
            Bar
        }

        public ProviderFactory(IProviderConfigurationService providerConfigurationService)
        {
            _providerConfigurationService = providerConfigurationService;
        }

        public Collection<IProvider> GetProviderInstances(SearchType searchType)
        {
            // Provider configuration service will read a XML/DB store to retrieve list of search providers applicable for a search type
            var providerList = _providerConfigurationService.GetProviderList(searchType);
            return new Collection<IProvider>(providerList.ForEach(x=> ServiceLocator.GetInstance(typeof(x))).ToList()) ;
        }
    }

What are my other options? I am currently using Unity for DI.

Jun Wei Lee
  • 1,002
  • 11
  • 25
  • Why do you need so many dependency object in the first place? – Hossain Muctadir Sep 26 '13 at 08:33
  • Depending on the search type, I will need to invoke a different set of providers. It is also a valid question on the factory pattern in general, as it's job is to create concrete instances which is against IoC principles. – Jun Wei Lee Sep 26 '13 at 08:37
  • What DI framework are you using? Ninject has a Factories extension that is fantastic for this. – Simon Whitehead Sep 26 '13 at 08:44
  • To add to my comment, in short the factory class will contain business logic of which providers to execute for a given search type. The task of executing the providers and aggregating the result is left to the consumer of the factory. The factory itself will be constructor injected. – Jun Wei Lee Sep 26 '13 at 08:44
  • @SimonWhitehead I am using Unity unfortunately. I have updated the question to reflect this. – Jun Wei Lee Sep 26 '13 at 08:46
  • It looks like what I wanted to avoid (injecting all possible providers into the factory) is kinda the only way to go, based on all the accepted answers Mark Seeman has with his Abstract Factory pattern answers. Like this: http://stackoverflow.com/a/2269105/1267655 – Jun Wei Lee Sep 26 '13 at 09:22

3 Answers3

2

An alternative is to pass a Func<Type, object> to the constructor and to implement the function through your container:

unity.RegisterInstance<Func<Type, object>>(t => unity.Resolve(t))

Then in your class:

public ProviderFactory(Func<Type, object> createFunc, IProviderConfigurationService pcs)
{
    _createFunc = createFunc; 
}

public Collection<IProvider> GetProviderInstances(SearchType searchType)
{
    var providerList = _providerConfigurationService.GetProviderList(searchType);
    return new Collection<IProvider>(providerList.Select(_createFunc).ToList());
}
Bojan Resnik
  • 7,320
  • 28
  • 29
  • Thanks @Bojan but that looks like it's just masking passing the DI container around which really falls into the gray area of service location, and especially since Func isn't that clear on what it's meant to do. – Jun Wei Lee Oct 02 '13 at 00:32
  • I agree it's not clear what it's meant to do immediately - but it's also easy to declare a delegate with more descriptive name (e.g. `public object delegate CreateProvider(Type providerType)`) and use that instead of `Func`. However, there is no service location here, as commonly understood. Any method will have to use DI container under the hood, and I don't think it's necessarily bad to do so, but using a delegate instead allows for more convenient testing. – Bojan Resnik Oct 03 '13 at 00:05
  • I think the delegate way hits the jackpot. It encapsulates the resolution of the type away from the rest of the code, and the delegate is named property, for extra points I have strongly typed the delegate to return IProvider so you'll not be able to abuse it as easily. – Jun Wei Lee Oct 03 '13 at 02:02
1

You are missing an abstraction.

Your ProviderFactory should implement an IProviderFactory abstraction. This way you can place that interface in a base library of your application and you can place the ProviderFactory implementation inside your Composition Root. For code that lives inside your composition root, it is okay to reference the DI library, and in that case you're not using service location.

Steven
  • 166,672
  • 24
  • 332
  • 435
  • So are you advocating that the concrete ProviderFactory live in the Global.asax (this is a WebAPI project) and references the DI container directly or am I reading something wrong? The ProviderFactory contains business logic and I'm not sure if that's appropriate there. – Jun Wei Lee Sep 26 '13 at 09:32
  • Yes, I'm advocating that all classes that reference the container live in the composition root. What I'm not advocating is that you place business logic inside your composition root. Make sure that business logic is placed in your application and doesn't reference the container. You should change your design accordingly. – Steven Sep 26 '13 at 09:50
  • Is there a simple example you can show? I'm not able to picture your solution that doesn't involve normal registration of IProviderFactory with my DI container at the bootstrapping stage, or passing the DI container to ProviderFactory through constructor injection. – Jun Wei Lee Sep 27 '13 at 00:11
  • Can you show a more complete implementation of your `ProviderFactory` that includes more business logic? If you do, I might be able to refactor it and show you an alternative implementation. – Steven Sep 27 '13 at 07:36
  • I have updated the question with a more complete example, but do note that I have not actually build the class yet. I am very curious on how to integrate the ProviderFactory into the composition root, as this will solve my non-static version of Udi Dahan's Domain Events, which involves bootstrapping a service locator into a static property. – Jun Wei Lee Sep 27 '13 at 12:52
  • @JunWeiLee: There are several ways to prevent business logic from leaking into your Composition Root. You could for instance extract business logic into a base class and create a derived class in your composition root. Or you can extract the business logic into a new service and let your implementation depend on that. You seem to have done the latter case and I don't see any business logic in your `ProviderFactory` class, just infrastructure. So I would say you're fine. – Steven Sep 27 '13 at 13:09
0

I have recently solved a very similar issue in my own code by using a DI framework. To satisfy Dependency Inversion, the factory constructor should accept an interface (as the other answers have said), but to get the framework to inject the right type is tricky without having a massive list of arguments detailing each possible concretion.

SimpleInjector allows you to register all concretions of a given abstraction with:

Container.RegisterCollection(typeof(IProvider), new [] {typeof(TKnown).Assembly,...});

Your XML could list the (possibly external) assemblies where the concretions are defined and you could build the assembly array from there. Then your factory just needs to accept them all and pick one, perhaps based on the searchType you mentioned.

public class ProviderFactory
{
    private List<IProvider> providers;
    public ProviderFactory(IEnumerable<IProvider> providers)
    {
        this.providers = providers.ToList();
    }

    public IProvider GetProvider(string searchType)
    {
        // using a switch here would open the factory to modification
        // which would break OCP
        var provider = providers.SingleOrDefault(concretion => concretion.GetType().Name == searchType);

        if (provider == null) throw new Exception("No provider found of that type.  Are you missing an assembly in the RegisterCollection for IProvider?");

        return provider;
    }

I know I'm way late to the party on this but assuming other folks don't see this approach as problematic, it might be useful.

stardotstar
  • 318
  • 2
  • 18