6
\$\begingroup\$

I have read some articles discussing about why static is bad, some of them refering static as not-testable, non-mockable, has state (non-stateless) and everything else.

However I have encountered some cases where I am forced to use static implementation, such as:

  • During using HttpContext.Current in Asp.Net, as well as Session
  • The HttpContext and Session sometimes contains configurations, which is needed to do some validations
  • Accessing some static objects in framework or third-party component
  • Maybe there are other situation where I don't yet encountered

Up until now I have handle those static methods using custom adapters (correct me if I misuse the term adapter here, or maybe it is better for provider), such as:

public class ConfigurationAdapter : IConfigurationAdapter
{
    public string CurrentUsername
    {
        get
        {
            return HttpContext.Current.User.Identity.Name;
        }
    }
}

Then the consumer will use the adapter like:

public class ConfigurationConsumer : IConfigurationConsumer
{
    public ConfigurationConsumer(IConfigurationAdapter adapter)
    {
        _adapter = adapter;
    }
    private readonly IConfigurationAdapter _adapter;

    public void DoSomething()
    {
        string userName = _adapter.CurrentUsername;
    }
}

I believe that this way I can get the benefits:

  • The adapter can be replaced by other implementation
  • The adapter can be mocked
  • The consumer does not directly dependent on HttpContext

However due to inexperienced in design, I am still wondering about:

  • Does this kind of implementation can really decouple the consumer with the static object?
  • Will it provide any problem in the future?
  • Is there any better design or pattern than this?

In this case I want to make sure that my application is testable, maintainable dan extendable. I believe performance is not effecting too much here, so it can be ignored.

Any kind of thoughts will be appreciated

\$\endgroup\$
6
\$\begingroup\$

I see no problem in such wrappers except they are kinda cumbersome to write. They provide a good way to decouple your code not only from static objects, but from the all the objects you cannot mock too (for example, sealed classes from 3rd party libraries). I use that kind of adapters myself and have little problem with them. It is important to have these classes as simple as possible (so that you don't need to test them) and only expose properties and methods you realy need.

Update: ctrucza suggests using a service locator in his answer. While it's a good solution when your class has many dependencies which leads to many constructor parameters, it makes unit-testing your class a little harder.

If you add some new dependency into your class and resolve it with some generic service locator's method then your code (including unit tests and production code) will compile just fine. You only realize that something is wrong when your working unit-tests will suddenly fail with null reference exception, or some other resolution exception your service locator provides. The constructor injection, on the other hand, gives you a clear knowledge of what dependencies a class using and if you miss some constructor parameter in your tests, the compiler will tell you.

One more thing. If you have a dozen dependencies in your class, may be the design of the class is wrong and refactoring is needed. I think that's really good indicator, which will be hidden if you are using a service locator pattern.

\$\endgroup\$
  • \$\begingroup\$ Thank you for sharing your experience. Looks like it is the good way to do it anyway. \$\endgroup\$ – Fendy Apr 18 '13 at 12:36
2
\$\begingroup\$

The only problem with this otherwise nice and simple approach is when you have many classes which need the adapters: you will have to pass the IConfigurationAdapter to each of them.

If this starts to be a problem I usually try to move towards a ServiceLocator:

public class Configuration 
{ 
    public static IConfigurationAdapter CurrentConfiguration { get; set; }
}

and use it like this:

public class ConfigurationConsumer
{
    public ConfigurationConsumer()
    {
        _adapter = Configuration.CurrentConfiguration;
    }
    private readonly IConfigurationAdapter _adapter;

    public void DoSomething()
    {
        string userName = _adapter.CurrentUsername;
    }
}

or like this:

public class ConfigurationConsumer
{
    public void DoSomething()
    {
        IConfigurationAdapter _adapter = Configuration.CurrentConfiguration;
        string userName = _adapter.CurrentUsername;
    }
}

(Sometimes I miss global variables :)

For a good discussion of injection vs. service locator see Martin Fowler's article here: http://martinfowler.com/articles/injection.html

\$\endgroup\$
  • 1
    \$\begingroup\$ This is a horrible suggestion. If you are going to assign it in the constructor, why not just inject it? \$\endgroup\$ – Jeff Vanzella Apr 18 '13 at 21:28
  • \$\begingroup\$ Although I agree Jeff, I guess ctrucza is suggesting that when a class A uses Class B which uses Class C which in turn needs to know about the current config by doing DI you will need to pass it through all the various levels even though Class B does not need to know about it?? I think that's the theory of the answer anyway \$\endgroup\$ – dreza Apr 18 '13 at 21:39
  • \$\begingroup\$ Not really, Class C should be created with CurrentConfig injected into it. Class C should then be injected into Class B, which is in turn injected into Class A. Therefore, A and B do not know about CurrentConfig. My main objection was the use of statics in a object oriented environment. They take way everything that is good about objects. \$\endgroup\$ – Jeff Vanzella Apr 18 '13 at 23:02
  • 1
    \$\begingroup\$ You can think of my proposal as a poor mans' service locator, an alternative to dependency injection. A thorough discussion of the topic can be found here: martinfowler.com/articles/injection.html \$\endgroup\$ – ctrucza Apr 19 '13 at 5:40
  • \$\begingroup\$ @ctrucza, you realy should have mentioned service locator and provide a link to Martin Fowler's blog in your answer, because the suggestion to use a static parameter is not very good indeed. It makes your code hard to test and increase coupling. I am sure you know about that, but some unexperienced readers may use this solution in their project only to regret about it later. \$\endgroup\$ – Andrei Zubov Apr 19 '13 at 6:14

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.