Rubberduck: Kill Ninject

[retailcoder]

Ninject has been identified as the root of all evil.

Removing Ninject has a few implications:

• Abstract factories will need concrete implementations.
• Bindings defined in RubberduckModule will need to be implemented manually (aka "Poor Man's DI"), until another DI/IoC framework is in place; we'll need to cleanly implement the composition root in the Rubberduck.Root namespace. This will be tedious, but definitely worth it.
• Interceptors won't work anymore: we'll lose performance timing of individual inspections. As a side-effect the GetType method of inspection types will start returning actual inspection types instead of Castle Windsor proxy types... which makes the recently-added Type property moot.
• Crash on exit: gone.
• Major performance boost on startup.

[MDoerner]

For some background why Ninject might be at least one root of our shutdown problems. According to this SO answer and its comments, Ninject makes absolutely no guarantees at which point it will release objects it manages. This is a problem for us since we bind quite a lot in singleton scope and there are some things we want to get rid off on shutdown, before the host forcefully unloads us.

Moreover, there are reports that Ninject actually reuses parts between its kernel instances. This might be one reason why unloading the add-in and then reloading it does not yield a fresh instance like on startup. (This results in a NullReferenceException because some control is not there anymore.)

As an added benefit of switching to another IoC container, we would get a speedup on load. According to this benchmark, Ninject is by far the slowest IoC container for C#.

[MDoerner]

Some more information on how Ninject lifetime management works

[MDoerner]

This article might actually contain a solution in Ninject. However, I still have to read it completely.

[ThunderFrame]

Begrudgingly concedes the "root of all evil" title

No amount of line continuation is going to get me that title back, is it?

[MDoerner]

To push this isssue a bit further I would like to present some pros and cons for two possible substitutes for Ninject from the long list in the benchmark in one of my previous comments.

Autofac: (Personally, I would go with this.)

_Pros:_

• Large userbase
• Good online documentation
• Straightforward syntax, although a bit backwards (You apply registrations to concrete types, not the other way around.)
• Supports most of Ninject's features directly or through emulation.
• Func support with resolution. (Caution: Parameters are matched by type, so that Func does not work. (runtime error) This respects lifetimes, so that a registration as singleton means that the Parameters are ignored the second time.)
• Multibinding and support for other wrappers like Lazy
• Support for open generics
• Strict separation between registration and resolution (All dependencies held by the resolution mechanism are released after building the container.)
• Good lifetime management
• Guarantees to dispose and release all disposables resolved from a lifetime scope on disposal of the lifetime scope, including the container itself.
• Registration can be separated into modules.
• Allows to implement custom RegistrationSouce classes for advanced registratoin logic.
• Resolution is thread-safe
• Average speed

_Cons:_

• Average speed
• Strict separation between registration and resolution (Forces us to rewrite some registration logic.)
• No WhenInjectedInto. (Needs to be emulated via WithParameters and keyed registration.)
• No auto-magic factories. (Either Func or factory delegates have to be used.)

DryIoC: (I only had a quick glace at the documentation.)

_Pros:_

• Fast
• Decent online documentation
• multibinding
• Wrapper support (Func, Lazy)
• Support for open generics
• Dynamic registration (More fexibility.)

_Cons:_

• Wrappers usually do not allow recursive resolution and if they do a quirky registration syntax has to be used.
• On disposal, the container unregisters all components, but disposes only singletons.
• Small user base
• Dynamic registration (Error prone; especially, unregistering does not behave as expected because of caching.)

To find more pros and cons, I would have to dig a bit more into the documentation. However, I will leave it with this.

For more possible choices, see the benchmark I mentioned in a previous comment.

[Hosch250]

We should also look into Castle Windsor and some of the others in DI in .NET.

[MDoerner]

Next IoC Container:

Castle Windsor: (The thing that potentially does absolutely everything.)

_Pros:_

• Large userbase
• Good online documentation
• Supports everything Ninject does (or it can be emulated) and more.
• Func<A,B,T> support but it is discuraged to use it. (Same restrictions as Autofac.)
• auto-magic factories based on interfaces (void members are interpreted as methods to release what has been resolved, except Dispose wich releases all.)
• Multibinding after loadeing the corresponding subresolver
• Support for open generics
• type-safe (You can only bind something to an interface it actually implements.)
• Guarantees to dispose and release all disposables resolved on disposal of the container, unless you explicitly deactivate it.
• Registration can be separated into installers.
• There are tons of extension points.
• Average speed

_Cons:_

• Average speed
• The syntax is quirky in places. (Registration by convention can actually quite confusing.)
• No WhenInjectedInto. (Needs to be emulated via ServiceOverwrite and named registration.)
• Singleton is the default lifestyle (This is uncommon and can cause strange problems if you forget it.)

Func<A,B,T> is discuraged because you have no way to release transient components. (Autufac offers the Owned<T> wrapper for this.

[Vogel612]

Not sure why we'd even want / need WhenInjectedInto if we had components that knew what they request ... currently WhenInjectedInto fills the role of a request granularization.

The solution I know from Java to this is to explicitly make the components that are created know a bit about what they need. In addition to the requested type, they can ask for something that conforms to a "hint" or a "name"

[rubberduck203]

@Vogel612 I'd argue that patterns of DI from Java-land aren't great. I've seen far too many annotations and hints in that world. Although, we're likely bikeshedding. I don't believe we were using WhenInjectedInto, were we?

Ok the topic of bikeshedding, I really doubt the selection of an IoC container is a critical decision here (Ignoring the Ninject debacle that necessitates this discussion), I'd just pick one and see how it goes. RD is generally well factored enough to accept any IoC container, even if it was hand rolled dependency injection at the entry point.

[Vogel612]

@rubberduck203 actually we are ... CommandBase is VersionCommand WhenInjectedExactlyInto. and then there's 12 other occurrences in the context root, mostly for IDockableToolWindow (or sth like that...)

The problem is not changing the IoC container, it's not having to change it again

[PeterMTaylor]

Would it be ideal to make a SlimDuckwithAutofac branch as an example or naming convention as SlimDuckwith(IoCname) for feasibility study each IoC to generate supportive data against the pro/cons @MDoerner offered.

[MDoerner]

We actually have one more place where we _should_ use WhenInjectedInto where we don't do it: the menus. There we do something worse, we resolve the submenues and then inject the results as fixed parameters.

[MDoerner]

If nobody has anything against it, I would like to try to port our registration logic to Castle Windsor (because of the auto-magic factories with proper release capabilities.)

As long as one does not try to add new conventions, even without looking at the documentation the syntax Castle Windsor uses should not be too confusing. So, the auto-magic factories are more useful from my point of view.