2
\$\begingroup\$

I'm using Autofac to create one of my classes inside of a static factory method Create. This code is working and I haven't noticed any side effects yet. Admittedly, I've only used it in unit-tests so far but I'm wondering whether there could be any issues with it later when I take it into production code?

public static ICommandLineExecutor Create(ILogger logger)
{
    var containerBuilder = new ContainerBuilder();

    containerBuilder
        .RegisterType<CommandLineTokenizer>()
        .As<ICommandLineTokenizer>();

    containerBuilder
        .RegisterType<CommandLineParser>()
        .As<ICommandLineParser>();

    containerBuilder
        .RegisterType<CommandParameterFactory>()
        .WithParameter(new TypedParameter(typeof(TypeConverter), CommandParameterFactory.DefaultConverter))
        .As<ICommandParameterFactory>();

    containerBuilder
        .RegisterType<CommandLineExecutor>()
        .WithParameter(new TypedParameter(typeof(ILogger), logger))
        .As<ICommandLineExecutor>();

    using (var container = containerBuilder.Build())
    using (var scope = container.BeginLifetimeScope())
    {
        return scope.Resolve<ICommandLineExecutor>();
    }
}
\$\endgroup\$
  • \$\begingroup\$ Actually I'm not sure if it would be better to ask this quesiton on Stack Overflow. What do you think? \$\endgroup\$ – t3chb0t Oct 29 '17 at 18:19
  • \$\begingroup\$ Is this generated code or code you wrote? I don't recall the specific meta, but there is some animosity versus generated code if I remember. Just a heads-up. \$\endgroup\$ – Mast Oct 29 '17 at 19:35
  • \$\begingroup\$ @Mast This isn't generated code. I wrote it myself. If you know how to generate something like this I'll be happy to take a look at it ;-) \$\endgroup\$ – t3chb0t Oct 29 '17 at 19:36
  • \$\begingroup\$ @Mast but thanks, if you think that this is generated then I take it as a compliment ;-] \$\endgroup\$ – t3chb0t Oct 29 '17 at 19:39
1
\$\begingroup\$

What are you registering CommandLineTokenizer etc for? this whole code block is equivalent to

return new CommandLineExecutor(logger);

If all these types are somehow used after all (inside CommandLineExecutor .ctor?), remember that everything created Per LifeTime scope will be disposed at exit out of using and everything created Per Dependency scope will become your responsibility to Dispose.

\$\endgroup\$
  • \$\begingroup\$ It's a bit more than that... the parser requires a tokenizer, the executor the parser and the factory so all these dependecies end up being used by the executor when it's get resolved. But thx for scope tip ;-) I'm still struggling to understand them and now I think I'm starting to finally get a picture. \$\endgroup\$ – t3chb0t Nov 4 '17 at 16:53
1
\$\begingroup\$

I can't say for sure without seeing the whole code, but I think this is violating the core principle of DI. You should never access the DI container anywhere except the composition root, which is where you resolve the startup class of your project and start doing the actual work. So, the registration code should be in the composition root where you do all your other registration, and you should just be able to inject an ICommandLineExecutor into whatever other constructors need it.

AutoFac automatically resolves types as their interfaces, so the last part of this code is redundant:

containerBuilder
    .RegisterType<CommandLineTokenizer>()
    .As<ICommandLineTokenizer>();

If CommandLineTokenizer implements ICommandLineTokenizer, then it will automatically be registered as both CommandLineTokenizer and ICommandLineTokenizer. You shouldn't need to register types with parameters explicitly; AutoFac will look at the parameters and inject them automatically. Actually, you really shouldn't be registering types explicitly; I don't know the exact syntax, but you can register a whole assembly at once.

Finally, if you really do need a different container here, then what you do is inject an ILifetimeScope instance into your class and do something like:

using (var newScope = _lifetimeScope.BeginLifetimeScope())
{
    var item = newScope.Resolve<Item>();
    // write your code using `item` here, and the new scope will be used
}
\$\endgroup\$
  • \$\begingroup\$ I use As<> because... I saw it in all Autofac examples and I think it's necessary if components implement more then one interface. Registering an entire assembly doesn't sound like a good idea ;-) I like to have everything under control and prefer to put it together myself. I'm using Autofac here because it's a factory method in a library and I didn't want to new everything myself and I don't want to put everything together each time but I like the idea about passing the outer scope to Create. I need to try this out... or even better, I'll create an Autofac-Module that I can register later \$\endgroup\$ – t3chb0t Nov 4 '17 at 18:36
  • \$\begingroup\$ Nope. AutoFac will register it as all interfaces it implements. \$\endgroup\$ – Hosch250 Nov 4 '17 at 18:37
  • \$\begingroup\$ Also, registering whole assemblies is usually a good idea--you should almost never be newing anything up, and it's a terrible pain manually registering tens or hundreds of types. On the other hand, I know the feeling of losing control; I felt like that when I was first learning about DI and IoC. \$\endgroup\$ – Hosch250 Nov 4 '17 at 18:39
  • \$\begingroup\$ It turns out that without .As<T> this won't work scope.Resolve<IEnumerable<ICommand>>(); when T is an ICommand registered only with .RegisterType<Copy>(). \$\endgroup\$ – t3chb0t Nov 5 '17 at 10:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.