Injecting Current User with Ninject this way - any disadvantages?

Question

I'm using this ninject bind:

kernel.Bind<ICurrentUser>().To<CurrentUser>()
    .InRequestScope()
    .WithConstructorArgument("principal", 
        context => (RolePrincipal HttpContext.Current.User);

In one of my decorators of the service layer I simply add "ICurrentUser currentUser" to the constructor to get the object and its working.

Are there any disadvantages by this implementation or is there a better way to implement his ambient context? For each request the user object is created - also if its an anonymous user.

Answer 1

The use for an Ambient Context is very limited and using constructor injection will usually yield the best results.

Think for instance how the use of an Ambient Context complicates unit testing. Using an Ambient Context would typically mean that you need to change that context in the test setup and remove it in the test teardown. But a unit test framework typically runs a set of tests on multiple threads (as MSTest does for instance) and when you use a static variable like this, it means that your tests influence each other.

[Update] Because this and other reasons, the book Dependency Injection in .NET, Second Edition calls Ambient Context an anti-pattern.

This all can simply be overcome by injecting all dependencies in the constructor. Or let's view it from a different standpoint. If your class depends on this service, why not inject into the constructor?

The only thing that's worrying me is the explicit constructor argument registration. This makes your configuration more brittle. Since your CurrentUser depends on a RolePrincipal, this justifies registering it directly:

kernel.Bind<ICurrentUser>().To<CurrentUser>().InRequestScope();

kernel.Bind<RolePrincipal>()
    .ToMethod(c => (RolePrincipal)HttpContext.Current.User);