8

I've many core class that require ISessionContext of the database, ILogManager for log and IService used for communicate with another services. I want use dependency injection for this class used by all core classes.

I've two possibile implementation. The core class that accept IAmbientContext with all the three class or inject for all class the three classes.

public interface ISessionContext 
{
    ...
}

public class MySessionContext: ISessionContext 
{
    ...
}

public interface ILogManager 
{

}

public class MyLogManager: ILogManager 
{
    ...
}

public interface IService 
{
    ...
}

public class MyService: IService
{
    ...
}

First solution:

public class AmbientContext
{
    private ISessionContext sessionContext;
    private ILogManager logManager;
    private IService service;

    public AmbientContext(ISessionContext sessionContext, ILogManager logManager, IService service)
    {
        this.sessionContext = sessionContext;
        this.logManager = logManager;
        this.service = service;
    }
}


public class MyCoreClass(AmbientContext ambientContext)
{
    ...
}

second solution(without ambientcontext)

public MyCoreClass(ISessionContext sessionContext, ILogManager logManager, IService service)
{
    ...
}

Wich is the best solution in this case?

  • What is "IService used for communicate with another services?" If IService represents a vague dependency on other services then it sounds like a service locator and shouldn't exist. Your class should should depend on interfaces that explicitly describe what their consumer will do with them. No class ever needs a service to provide access to a service. A class needs a dependency that does something specific that the class needs. – Scott Hannen Jan 10 '18 at 14:18
4

"Best" is far too subjective here. As is common with such decisions, it's a trade-off between two equally valid ways of achieving something.

If you create an AmbientContext and inject that into many classes, you are potentially providing more information to each of them than they need (eg class Foo may only use ISessionContext, but is being told about ILogManager and ISession too).

If you pass each in via a parameter, then you tell each class about only those things that it needs to know about. But, the number of parameters can quickly grow and you may find you have far too many constructors and methods with many, highly repeated, parameters, which could be simplified via a context class.

So it's a case of balancing the two and choosing the appropriate one for your circumstances. If you only have one class and three parameters, I'd personally not bother with AmbientContext. For me, the tipping point would likely be four parameters. But that is pure opinion. Your tipping point will likely be different to mine, so go with what feels right to you.

4

The terminology from the question doesn't really match with the example code. The Ambient Context is a pattern used to grab a dependency from any class in any module as easy as possible, without polluting every class to accept the dependency's interface, but still keeping the idea of inversion of control. Such dependencies are usually dedicated to logging, security, session management, transactions, caching, audit so to any cross cutting concern in that application. It's somehow annoying to add an ILogging, ISecurity, ITimeProvider to constructors and most of the time not all classes need all at the same time, so I understand your need.

What if the lifetime of the ISession instance is different to the ILogger one? Maybe the ISession instance should be created on every request and the ILogger once. So having all these dependencies governed by one object that is not the container itself doesn't look the right choice because of all these problems with lifetime management and localization and others described in this thread.

The IAmbientContext in the question doesn't solve the problem of not polluting every constructor. You still have to use it in the constructor signature, sure, one time only this time.

So the easiest way is NOT use constructor injection or any other injection mechanism to deal with cross cutting dependencies, but using a static call. We are actually see this pattern quite often, implemented by the framework itself. Check Thread.CurrentPrincipal which is a static property that returns an implementation of the IPrincipal interface. It's also settable so you can change the implementation if you like so, thus you aren't coupled to it.

MyCore looks now something like

public class MyCoreClass
{
    public void BusinessFeature(string data)
    {
        LoggerContext.Current.Log(data);

        _repository.SaveProcessedData();

        SessionContext.Current.SetData(data);
        ...etc
    }
}

This pattern and possible implementations have been described in detail by Mark Seemann in this article. There might be implementations that rely on the IoC container itself you use.

You want to avoid AmbientContext.Current.Logger, AmbientContext.Current.Session for the same reasons as described above.

But you have other options to solve this problem: use decorators, dynamic interception if your container has this capability or AOP. The Ambient Context should be the last resort because of the fact its clients hide their dependencies through it. I would still use Ambient Context if the interface it really mimics my impulse to use a static dependency like DateTime.Now or ConfigurationManager.AppSettings and this need raises quite often. But in the end the constructor injection might not be such a bad idea to get these ubiquitous dependencies.

2

The second (without the interface wrapper)

Unless there is some interaction between the various services which needs encapsulating in an intermediate class, it only complicates your code and limits flexibility when you introduce the 'interface of interfaces'

2

I'd avoid the AmbientContext.

First, if the class depends on AmbientContext then you don't actually know what it does. You have to look at its use of that dependency to figure out which of its nested dependencies it uses. You also can't look at the number of dependencies and tell if your class is doing too much because one of those dependencies might actually represent several nested dependencies.

Second, if you're using it to avoid multiple constructor dependencies then this approach will encourage other developers (including yourself) to add new members to that ambient context class. Then the first problem is compounded.

Third, mocking the dependency on AmbientContext is more difficult because you have to figure out in each case whether to mock all of its members or just the ones you need, and then set up a mock that returns those mocks (or test doubles.) That makes your unit tests harder to write, read, and maintain.

Fourth, it lacks cohesion and violates the Single Responsibility Principle. That's why it has a name like "AmbientContext," because it does lots of unrelated things and there's no way to name it according to what it does.

And it probably violates the Interface Segregation Principle by introducing interface members into classes that don't need them.

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.