3

I'm trying to create a generic factory I can call to instantiate a class and its dependencies using Ninject constructor injection. It seems to work great, but its not sitting well with me, I don't know if that's because its the first time I've used generics and an IoC container, but I think my approach is flawed. Rather than explain ill just dump my simple test console app.

Farm.cs

class Farm
{
    private readonly IAnimal _animal;
    private readonly IVehicle _vehicle;

    public Farm(IAnimal animal, IVehicle vehicle)
    {
        _animal = animal;
        _vehicle = vehicle;
    }

    public void Listen()
    {
        _animal.Speak();
        _vehicle.Run();
    }
}

program.cs

class Program
{
    static void Main(string[] args)
    {
        var farm = new NinjectFactory<Farm>().GetInstance();

        farm.Listen();

        Console.Read();
    }
}

NinjectFactory.cs

class NinjectFactory<T>
{
    public T GetInstance()
    {
        var kernel = new StandardKernel(new IoCModule());

        return kernel.Get<T>();
    }
}

NinjectModule.cs

class IoCModule : NinjectModule 
{
    public override void Load()
    {
        Bind<IAnimal>().To<Dog>();
        Bind<IVehicle>().To<Tractor>();
    }
}

Any ideas/feedback would be greatly appreciated, thanks.

Tom Riley
  • 1,701
  • 2
  • 21
  • 43
  • 4
    That basically is the [service locator anti-pattern](http://blog.ploeh.dk/2010/02/03/ServiceLocatorIsAnAntiPattern.aspx). Probably that's why it feels strange. – Daniel Hilgarth Oct 11 '12 at 15:39
  • Looks perfectly fine to me... if you switch to automatic constructor dependency injection then this is what it looks like. What exactly is "flawed" in your opinion? – Paul Michalik Oct 11 '12 at 15:41
  • Give more example where you use Factory? – cuongle Oct 11 '12 at 18:31
  • @PaulMichalik I think the main thing that doesn't sit right it the fact I am creating a new kernel every time I use the factory. But I wasn't sure if the whole approach was flawed. If nothing is glaringly obvious as a no no then I'll run with this and see what I can do with it. – Tom Riley Oct 11 '12 at 21:33
  • @DanielHilgarth I can see what you mean, but what I'm now struggling to get straight in my head is how this differs from DI with an IoC container? Don't both hide potential issues from compile time? – Tom Riley Oct 11 '12 at 21:52

1 Answers1

2

I'm trying to create a generic factory I can call to instantiate a class and its dependencies using Ninject constructor injection

The Ninject Kernel (or any container for that matter) IS a generic factory. You are hiding that generic factory behind a generic factory. You can simply do this:

private static StandardKernel kernel;

static void Main(string[] args)
{
    Bootstrap();

    // Resolve the application's root type
    // by using the container directly.
    var farm = kernel.Get<Farm>();

    // Operate on the root type
    farm.Listen();

    Console.Read();
}

private static Kernel Bootstrap()
{
    kernel = new StandardKernel();

    kernel.Bind<IAnimal>().To<Dog>();
    kernel.Bind<IVehicle>().To<Tractor>();
    kernel.Bind<Farm>().ToSelf();
}

If your idea is to use your generic factory to hide the container from the application, this means that application code depends on that static factory. This is a big no-no. All types should be designed around constructor injection, and injecting a generic factory is the same as injecting the kernel itself into a type. This leads to code that is hard to maintain, hard to test, and hard to be verified.

Community
  • 1
  • 1
Steven
  • 166,672
  • 24
  • 332
  • 435
  • That's a very good point. I was trying to avoid having to use the kernel throughout the application but i was just recreating the same problem. My previous experience with IoC was using Ninject's mvc extension, which elevates the need to call the kernel directly. I think i'll go with poor mans DI (Bastard Injection) until I fully understand what I'm doing, It'll be easier to implement a container once i understand it compared to fixing a broken implementation. – Tom Riley Oct 12 '12 at 08:15