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?