IoC Registration: Convention over Configuration

Question

The rubberduck project has reached a turning point. The core features are implemented (except the SmartIndenter embedding - we're keeping that for 2.0), the next few releases will probably just build on the architecture in place.

One of the biggest architectural issues, is tight coupling, with dependencies being new'd up just about anywhere they're needed. Sometimes dependencies of dependencies are being constructor-injected... it's time for a major cleanup/refactoring.

The solution is proper Inversion of Control and Dependency Injection - and Rubberduck's dependency graph is getting quite extensive, so we're going to be using an IoC container. I picked ninject because I love its neat API, and how its lets you register the types by convention rather than by configuration.

Here's Rubberduck's entry point class, where the composition root lives. There used to be a private field there, private App _app; - it was removed because, well, everything works without it. On the plus side, I like that all the class cares about is the IKernel. On the other hand, I can't help thinking something isn't right - if all I need to do for everything to work, is to instantiate the App class, doesn't it mean I have a constructor doing way too much work?

using System;
using System.ComponentModel;
using System.Runtime.InteropServices;
using System.Windows.Forms;
using Extensibility;
using Microsoft.Vbe.Interop;
using Ninject;
using Ninject.Extensions.Factory;
using Rubberduck.Root;
using Rubberduck.UI;

namespace Rubberduck
{
    [ComVisible(true)]
    [Guid(ClassId)]
    [ProgId(ProgId)]
    [EditorBrowsable(EditorBrowsableState.Never)]
    // ReSharper disable once InconsistentNaming // note: underscore prefix hides class from COM API
    public class _Extension : IDTExtensibility2
    {
        private const string ClassId = "8D052AD8-BBD2-4C59-8DEC-F697CA1F8A66";
        private const string ProgId = "Rubberduck.Extension";

        private readonly IKernel _kernel = new StandardKernel(new FuncModule());

        public void OnAddInsUpdate(ref Array custom)
        {
        }

        public void OnBeginShutdown(ref Array custom)
        {
        }

        // ReSharper disable InconsistentNaming // blame Microsoft
        public void OnConnection(object Application, ext_ConnectMode ConnectMode, object AddInInst, ref Array custom)
        {
            try
            {
                var conventions = new RubberduckConventions(_kernel);
                conventions.Apply((VBE)Application, (AddIn)AddInInst);
                _kernel.Get<App>();
            }
            catch (Exception exception)
            {
                MessageBox.Show(exception.Message, RubberduckUI.RubberduckLoadFailure, MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
        }

        public void OnStartupComplete(ref Array custom)
        {

        }

        public void OnDisconnection(ext_DisconnectMode RemoveMode, ref Array custom)
        {
            _kernel.Dispose();
        }
    }
}

I'm not going to post the App class, because its refactoring is still under extensive work in progress, and it's rather uninteresting for the purpose of this question.

The RubberduckConventions class however, is exactly what this post is about:

namespace Rubberduck.Root
{
    public class RubberduckConventions
    {
        private readonly IKernel _kernel;

        public RubberduckConventions(IKernel kernel)
        {
            _kernel = kernel;
        }

        /// <summary>
        /// Configures the IoC <see cref="IKernel"/>.
        /// </summary>
        /// <param name="vbe">The <see cref="VBE"/> instance provided by the host application.</param>
        /// <param name="addin">The <see cref="AddIn"/> instance provided by the host application.</param>
        public void Apply(VBE vbe, AddIn addin)
        {
            _kernel.Bind<App>().ToSelf();

            // bind VBE and AddIn dependencies to host-provided instances.
            _kernel.Bind<VBE>().ToConstant(vbe);
            _kernel.Bind<AddIn>().ToConstant(addin);

            BindCodeInspectionTypes();

            var assemblies = new[]
            {
                Assembly.GetExecutingAssembly(),
                Assembly.GetAssembly(typeof(IHostApplication)),
                Assembly.GetAssembly(typeof(IRubberduckParser))
            };

            ApplyAllInterfacesConvention(assemblies);
            ApplyAbstractFactoryConvention(assemblies);
        }

        // note: binds all interfaces
        private void ApplyAllInterfacesConvention(IEnumerable<Assembly> assemblies)
        {
            _kernel.Bind(t => t.From(assemblies)
                .SelectAllClasses()
                // inspections & factories have their own binding rules
                .Where(type => !type.Name.EndsWith("Factory") && !type.GetInterfaces().Contains(typeof(IInspection)))
                .BindAllInterfaces()
                .Configure(binding => binding.InCallScope()));
        }

        // note convention: abstract factory interface names end with "Factory".
        private void ApplyAbstractFactoryConvention(IEnumerable<Assembly> assemblies)
        {
            _kernel.Bind(t => t.From(assemblies)
                .SelectAllInterfaces()
                .Where(type => type.Name.EndsWith("Factory"))
                .BindToFactory()
                .Configure(binding => binding.InSingletonScope()));
        }

        // note: IInspection implementations are discovered in the Rubberduck assembly via reflection.
        private void BindCodeInspectionTypes()
        {
            var inspections = Assembly.GetExecutingAssembly()
                                      .GetTypes()
                                      .Where(type => type.GetInterfaces().Contains(typeof (IInspection)));

            // multibinding for IEnumerable<IInspection> dependency
            foreach (var inspection in inspections)
            {
                _kernel.Bind<IInspection>().To(inspection).InSingletonScope();
            }
        }
    }
}

The bindings work beautifully, and the host Office app doesn't crash when I close it normally, which is pretty encouraging. Is this a good/clean way to go about configuring ninject with conventions?

Answer 1

When you have very "generic" convention, such as these, you need to make sure there's is a way to create a non-convention binding in case you need one.

There are several ways to achieve this:

1. provide a way to exclude a type from the convention (for example, you could add a [DoNotBindByConvention] attribute to these types. Not very neat but works.)
2. you could include the type in the convention, create a binding, and then replace it by using Rebind<T>.
   • requires a strict sequence of creating the bindings. First the convention, then all other bindings.
   • if you have a "multi binding", i.e. something you want to inject as T[], List<T>,..., or by calling IResolutionRoot.GetAll<T>(), rebind doesn't work.

---

Generally, such generic conventions are somewhat brittle / dangerous. As your example shows you already have to exclude some types (factories...) from some convention. There's potential that the number of these special cases increases and thus conventions become very complicated. Maybe not for you, in case you're writing these, but how will go you about communicating these to your fellow developers? That's usually an important reason not to use such broad conventions.

Hints

Some containers - like Ninject and AutoFac - also provide a "Module" concept. A "Module" can be used to group contain bindings /-conventions specific to an area. One can load specific modules or modules of a whole assembly. More on Ninject Modules