ploeh blog danish software design

In my previous posts I demonstrated interaction-based unit tests that verify that a pizza is correctly being added to a shopping basket. An alternative is a state-based test where we examine the contents of the shopping basket after exercising the SUT. Here's an initial attempt:

[TestMethod]
public void AddWillAddToBasket()
{
    // Fixture setup
    var fixture = new Fixture();
    fixture.Register<IPizzaMap>(
        fixture.CreateAnonymous<PizzaMap>);
 
    var basket = fixture.Freeze<Basket>();
 
    var pizza = fixture.CreateAnonymous<PizzaPresenter>();
 
    var sut = fixture.CreateAnonymous<BasketPresenter>();
    // Exercise system
    sut.Add(pizza);
    // Verify outcome
    Assert.IsTrue(basket.Pizze.Any(p => 
        p.Name == pizza.Name), "Basket has added pizza.");
    // Teardown
}

In this case the assertion examines the Pizze collection (you did know that the plural of pizza is pizze, right?) of the frozen Basket to verify that it contains the added pizza.

The tricky part is that the Pizze property is a collection of Pizza instances, and not PizzaPresenter instances. The injected IPizzaMap instance is responsible for mapping from PizzaPresenter to Pizza, but since we are rewriting this as a state-based test, I thought it would also be interesting to write the test without using Moq. Instead, we can use the real implementation of IPizzaMap, but this means that we must instruct AutoFixture to map from the abstract IPizzaMap to the concrete PizzaMap.

We see that happening in this line of code:

fixture.Register<IPizzaMap>(
    fixture.CreateAnonymous<PizzaMap>);

Notice the method group syntax: we pass in a delegate to the CreateAnonymous method, which means that every time the fixture is asked to create an IPizzaMap instance, it invokes CreateAnonymous<PIzzaMap>() and uses the result.

This is, obviously, a general-purpose way in which we can map compatible types, so we can write an extension method like this one:

public static void Register<TAbstract, TConcrete>(
    this Fixture fixture) where TConcrete : TAbstract
{
    fixture.Register<TAbstract>(() =>
        fixture.CreateAnonymous<TConcrete>());
}

(I'm slightly undecided on the name of this method. Map might be a better name, but I just like the equivalence to some common DI Containers and their Register methods.) Armed with this Register overload, we can now rewrite the previous Register statement like this:

fixture.Register<IPizzaMap, PizzaMap>();

It's the same amount of code lines, but I find it slightly more succinct and communicative.

The real point of this blog post, however, is that you can map abstract types to concrete types, and that you can always write extension methods to encapsulate your own AutoFixture idioms.

Comments

AutoFixture 1.1 Release Candidate 1 is now available on the CodePlex site.

Users are encouraged to evaluate this RC and submit feedback. If no bugs or issues are reported within the next week, we will promote RC1 to version 1.1.

The release page has more details about this particular release.

My previous post about AutoFixture's Freeze functionality included this little piece of code that I didn't discuss:

var mapMock = new Mock<IPizzaMap>();
fixture.Register(mapMock.Object);

In case you were wondering, this is Moq interacting with AutoFixture. Here we create a new Test Double and register it with the fixture. This is very similar to AutoFixture's built-in Freeze functionality, with the difference that we register an IPizzaMap instance, which isn't the same as the Mock<IPizzaMap> instance.

It would be nice if we could simply freeze a Test Double emitted by Moq, but unfortunately we can't directly use the Freeze method, since Freeze<Mock<IPizzaMap>>() would freeze a Mock<IPizzaMap>, but not IPizzaMap itself. On the other hand, Freeze<IPizzaMap>() wouldn't work because we haven't told the fixture how to create IPizzaMap instances, but even if we had, we wouldn't have a Mock<IPizzaMap> against which we could call Verify.

On the other hand, it's trivial to write an extension method to Fixture:

public static Mock<T> FreezeMoq<T>(this Fixture fixture)
    where T : class
{
    var td = new Mock<T>();
    fixture.Register(td.Object);
    return td;
}

I chose to call the method FreezeMoq to indicate its affinity with Moq.

We can now rewrite the unit test from the previous post like this:

[TestMethod]
public void AddWillPipeMapCorrectly_FreezeMoq()
{
    // Fixture setup
    var fixture = new Fixture();
 
    var basket = fixture.Freeze<Basket>();
    var mapMock = fixture.FreezeMoq<IPizzaMap>();
 
    var pizza = fixture.CreateAnonymous<PizzaPresenter>();
 
    var sut = fixture.CreateAnonymous<BasketPresenter>();
    // Exercise system
    sut.Add(pizza);
    // Verify outcome
    mapMock.Verify(m => m.Pipe(pizza, basket.Add));
    // Teardown
}

You may think that saving only a single line of code may not be that big a deal, but if you also need to perform Setups on the Mock, or if you have several different Mocks to configure, you may appreciate the encapsulation. I know I sure do.

Comments

// does not work
var td = new Mock();
fixture.Register(td.Object);

// should work
fixture.Inject(td.Object);

fixture.Register(() => td.object);

In my previous blog post, I introduced AutoFixture's Freeze feature, but the example didn't fully demonstrate the power of the concept. In this blog post, we will turn up the heat on the frozen pizza a notch.

The following unit test exercises the BasketPresenter class, which is simply a wrapper around a Basket instance (we're doing a pizza online shop, if you were wondering). In true TDD style, I'll start with the unit test, but I'll post the BasketPresenter class later for reference.

[TestMethod]
public void AddWillPipeMapCorrectly()
{
    // Fixture setup
    var fixture = new Fixture();
 
    var basket = fixture.Freeze<Basket>();
 
    var mapMock = new Mock<IPizzaMap>();
    fixture.Register(mapMock.Object);
 
    var pizza = fixture.CreateAnonymous<PizzaPresenter>();
 
    var sut = fixture.CreateAnonymous<BasketPresenter>();
    // Exercise system
    sut.Add(pizza);
    // Verify outcome
    mapMock.Verify(m => m.Pipe(pizza, basket.Add));
    // Teardown
}

The interesting thing in the above unit test is that we Freeze a Basket instance in the fixture. We do this because we know that the BasketPresenter somehow wraps a Basket instance, but we trust the Fixture class to figure it out for us. By telling the fixture instance to Freeze the Basket we know that it will reuse the same Basket instance throughout the entire test case. That includes the call to CreateAnonymous<BasketPresenter>.

This means that we can use the frozen basket instance in the Verify call because we know that the same instance will have been reused by the fixture, and thus wrapped by the SUT.

When you stop to think about this on a more theoretical level, it fortunately makes a lot of sense. AutoFixture's terminology is based upon the excellent book xUnit Test Patterns, and a Fixture instance pretty much corresponds to the concept of a Fixture. This means that freezing an instance simply means that a particular instance is constant throughout that particular fixture. Every time we ask for an instance of that class, we get back the same frozen instance.

In DI Container terminology, we just changed the Basket type's lifetime behavior from Transient to Singleton.

For reference, here's the BasketPresenter class we're testing:

public class BasketPresenter
{
    private readonly Basket basket;
    private readonly IPizzaMap map;
 
    public BasketPresenter(Basket basket, IPizzaMap map)
    {
        if (basket == null)
        {
            throw new ArgumentNullException("basket");
        }
        if (map == null)
        {
            throw new ArgumentNullException("map");
        }
 
        this.basket = basket;
        this.map = map;
    }
 
    public void Add(PizzaPresenter presenter)
    {
        this.map.Pipe(presenter, this.basket.Add);
    }
}

If you are wondering about why this is interesting at all, and why we don't just pass in a Basket through the BasketPresenter's constructor, it's because we are using AutoFixture as a SUT Factory. We want to be able to refactor BasketPresenter (and in this case particularly its constructor) without breaking a lot of existing tests. The level of indirection provided by AutoFixture gives us just that ability because we never directly invoke the constructor.

Coming up: more fun with the Freeze concept!

Comments

One of the important points of AutoFixture is to hide away all the boring details that you don't care about when you are writing a unit test, but that the compiler seems to insist upon. One of these details is how you create a new instance of your SUT.

Every time you create an instance of your SUT using its constructor, you make it more difficult to refactor that constructor. This is particularly true when it comes to Constructor Injection because you often need to define a Test Double in each unit test, but even for primitive types, it's more maintenance-friendly to use a SUT Factory.

AutoFixture is a SUT Factory, so we can use it to create instances of our SUTs. However, how do we correlate constructor parameters with variables in the test when we will not use the constructor directly?

This is where the Freeze method comes in handy, but let's first examine how to do it with the core API methods CreateAnonymous and Register.

Imagine that we want to write a unit test for a Pizza class that takes a name in its constructor and exposes that name as a property. We can write this test like this:

[TestMethod]
public void NameIsCorrect()
{
    // Fixture setup
    var fixture = new Fixture();
 
    var expectedName = fixture.CreateAnonymous("Name");
    fixture.Register(expectedName);
 
    var sut = fixture.CreateAnonymous<Pizza>();
    // Exercise system
    string result = sut.Name;
    // Verify outcome
    Assert.AreEqual(expectedName, result, "Name");
    // Teardown
}

The important lines are these two:

var expectedName = fixture.CreateAnonymous("Name");
fixture.Register(expectedName);

What's going on here is that we create a new string, and then we subsequently Register this string so that every time the fixture instance is asked to create a string, it will return this particular string. This also means that when we ask AutoFixture to create an instance of Pizza, it will use that string as the constructor parameter.

It turned out that we used this coding idiom so much that we decided to encapsulate it in a convenience method. After some debate we arrived at the name Freeze, because we essentially freeze a single anonymous variable in the fixture, bypassing the default algorithm for creating new instances. Incidentally, this is one of very few methods in AutoFixture that breaks CQS, but although that bugs me a little, the Freeze concept has turned out to be so powerful that I live with it.

Here is the same test rewritten to use the Freeze method:

[TestMethod]
public void NameIsCorrect_Freeze()
{
    // Fixture setup
    var fixture = new Fixture();
    var expectedName = fixture.Freeze("Name");
    var sut = fixture.CreateAnonymous<Pizza>();
    // Exercise system
    string result = sut.Name;
    // Verify outcome
    Assert.AreEqual(expectedName, result, "Name");
    // Teardown
}

In this example, we only save a single line of code, but apart from that, the test also becomes a little more communicative because it explicitly calls out that this particular string is frozen.

However, this is still a pretty lame example, but while I intend to follow up with a more complex example, I wanted to introduce the concept gently.

For completeness sake, here's the Pizza class:

public class Pizza
{
    private readonly string name;
 
    public Pizza(string name)
    {
        if (name == null)
        {
            throw new ArgumentNullException("name");
        }
 
        this.name = name;
    }
 
    public string Name
    {
        get { return this.name; }
    }
}

As you can see, the test simply verifies that the constructor parameter is echoed by the Name property, and the Freeze method makes this more explicit while we still enjoy the indirection of not invoking the constructor directly.

Comments

new Pizza(string fancyName, string boringName)

As part of the Copenhagen .NET User Group (CNUG) winter and early autumn schedule, I'll be giving a talk on (slightly advanced) TDD.

The talk will be in Danish and takes place April 15th, 2010. More details and sign-up here.

Service Locator is a well-known pattern, and since it was described by Martin Fowler, it must be good, right?

No, it's actually an anti-pattern and should be avoided.

Let's examine why this is so. In short, the problem with Service Locator is that it hides a class' dependencies, causing run-time errors instead of compile-time errors, as well as making the code more difficult to maintain because it becomes unclear when you would be introducing a breaking change.

OrderProcessor example #

As an example, let's pick a hot topic in DI these days: an OrderProcessor. To process an order, the OrderProcessor must validate the order and ship it if valid. Here's an example using a static Service Locator:

public class OrderProcessor : IOrderProcessor
{
    public void Process(Order order)
    {
        var validator = Locator.Resolve<IOrderValidator>();
        if (validator.Validate(order))
        {
            var shipper = Locator.Resolve<IOrderShipper>();
            shipper.Ship(order);
        }
    }
}

The Service Locator is used as a replacement for the new operator. It looks like this:

public static class Locator
{
    private readonly static Dictionary<Type, Func<object>>
        services = new Dictionary<Type, Func<object>>();
 
    public static void Register<T>(Func<T> resolver)
    {
        Locator.services[typeof(T)] = () => resolver();
    }
 
    public static T Resolve<T>()
    {
        return (T)Locator.services[typeof(T)]();
    }
 
    public static void Reset()
    {
        Locator.services.Clear();
    }
}

We can configure the Locator using the Register method. A ‘real' Service Locator implementation would be much more advanced than this, but this example captures the gist of it.

This is flexible and extensible, and it even supports replacing services with Test Doubles, as we will see shortly.

Given that, then what could be the problem?

API usage issues #

Let's assume for a moment that we are simply consumers of the OrderProcessor class. We didn't write it ourselves, it was given to us in an assembly by a third party, and we have yet to look at it in Reflector.

This is what we get from IntelliSense in Visual Studio:

Okay, so the class has a default constructor. That means I can simply create a new instance of it and invoke the Process method right away:

var order = new Order();
var sut = new OrderProcessor();
sut.Process(order);

Alas, running this code surprisingly throws a KeyNotFoundException because the IOrderValidator was never registered with Locator. This is not only surprising, it may be quite baffling if we don't have access to the source code.

By perusing the source code (or using Reflector) or consulting the documentation (ick!) we may finally discover that we need to register an IOrderValidator instance with Locator (a completely unrelated static class) before this will work.

In a unit test test, this can be done like this:

var validatorStub = new Mock<IOrderValidator>();
validatorStub.Setup(v => v.Validate(order)).Returns(false);
Locator.Register(() => validatorStub.Object);

What is even more annoying is that because the Locator's internal store is static, we need to invoke the Reset method after each unit test, but granted: that is mainly a unit testing issue.

All in all, however, we can't reasonably claim that this sort of API provides a positive developer experience.

Maintenance issues #

While this use of Service Locator is problematic from the consumer's point of view, what seems easy soon becomes an issue for the maintenance developer as well.

Let's say that we need to expand the behavior of OrderProcessor to also invoke the IOrderCollector.Collect method. This is easily done, or is it?

public void Process(Order order)
{
    var validator = Locator.Resolve<IOrderValidator>();
    if (validator.Validate(order))
    {
        var collector = Locator.Resolve<IOrderCollector>();
        collector.Collect(order);
        var shipper = Locator.Resolve<IOrderShipper>();
        shipper.Ship(order);
    }
}

From a pure mechanistic point of view, that was easy - we simply added a new call to Locator.Resolve and invoke IOrderCollector.Collect.

Was this a breaking change?

This can be surprisingly hard to answer. It certainly compiled fine, but broke one of my unit tests. What happens in a production application? The IOrderCollector interface may already be registered with the Service Locator because it is already in use by other components, in which case it will work without a hitch. On the other hand, this may not be the case.

The bottom line is that it becomes a lot harder to tell whether you are introducing a breaking change or not. You need to understand the entire application in which the Service Locator is being used, and the compiler is not going to help you.

Variation: Concrete Service Locator #

Can we fix these issues in some way?

One variation commonly encountered is to make the Service Locator a concrete class, used like this:

public void Process(Order order)
{
    var locator = new Locator();
    var validator = locator.Resolve<IOrderValidator>();
    if (validator.Validate(order))
    {
        var shipper = locator.Resolve<IOrderShipper>();
        shipper.Ship(order);
    }
}

However, to be configured, it still needs a static in-memory store:

public class Locator
{
    private readonly static Dictionary<Type, Func<object>>
        services = new Dictionary<Type, Func<object>>();
 
    public static void Register<T>(Func<T> resolver)
    {
        Locator.services[typeof(T)] = () => resolver();
    }
 
    public T Resolve<T>()
    {
        return (T)Locator.services[typeof(T)]();
    }
 
    public static void Reset()
    {
        Locator.services.Clear();
    }
}

In other words: there's no structural difference between the concrete Service Locator and the static Service Locator we already reviewed. It has the same issues and solves nothing.

Variation: Abstract Service Locator #

A different variation seems more in line with true DI: the Service Locator is a concrete class implementing an interface.

public interface IServiceLocator
{
    T Resolve<T>();
}
 
public class Locator : IServiceLocator
{
    private readonly Dictionary<Type, Func<object>> services;
 
    public Locator()
    {
        this.services = new Dictionary<Type, Func<object>>();
    }
 
    public void Register<T>(Func<T> resolver)
    {
        this.services[typeof(T)] = () => resolver();
    }
 
    public T Resolve<T>()
    {
        return (T)this.services[typeof(T)]();
    }
}

With this variation it becomes necessary to inject the Service Locator into the consumer. Constructor Injection is always a good choice for injecting dependencies, so OrderProcessor morphs into this implementation:

public class OrderProcessor : IOrderProcessor
{
    private readonly IServiceLocator locator;
 
    public OrderProcessor(IServiceLocator locator)
    {
        if (locator == null)
        {
            throw new ArgumentNullException("locator");
        }
 
        this.locator = locator;
    }
 
    public void Process(Order order)
    {
        var validator =
            this.locator.Resolve<IOrderValidator>();
        if (validator.Validate(order))
        {
            var shipper =
                this.locator.Resolve<IOrderShipper>();
            shipper.Ship(order);
        }
    }
}

Is this good, then?

From a developer perspective, we now get a bit of help from IntelliSense:

What does this tell us? Nothing much, really. Okay, so OrderProcessor needs a ServiceLocator - that's a bit more information than before, but it still doesn't tell us which services are needed. The following code compiles, but crashes with the same KeyNotFoundException as before:

var order = new Order();
var locator = new Locator();
var sut = new OrderProcessor(locator);
sut.Process(order);

From the maintenance developer's point of view, things don't improve much either. We still get no help if we need to add a new dependency: is it a breaking change or not? Just as hard to tell as before.

Summary #

The problem with using a Service Locator isn't that you take a dependency on a particular Service Locator implementation (although that may be a problem as well), but that it's a bona-fide anti-pattern. It will give consumers of your API a horrible developer experience, and it will make your life as a maintenance developer worse because you will need to use considerable amounts of brain power to grasp the implications of every change you make.

The compiler can offer both consumers and producers so much help when Constructor Injection is used, but none of that assistance is available for APIs that rely on Service Locator.

You can read more about DI patterns and anti-patterns in my book.

Update 2014-05-20: Another way to explain the negative aspects of Service Locator is that it violates SOLID.

Update 2015-10-26: The fundamental problem with Service Locator is that it violates encapsulation.

Comments

public class MyService : ISomeService
{
    private readonly ICommonService blue;
    private readonly ICommonService red;
 
    public MyService(ICommonService blue, ICommonService red)
    {
        if (blue == null)
            throw new ArgumentNullException(nameof(blue));
        if (red == null)
            throw new ArgumentNullException(nameof(red));
        
        this.blue = blue;
        this.red = red;
    }
 
    // Members that DO something can go here...
}

public class BlueService : ICommonService
{
    private readonly ICommonService green;
 
    public BlueService(ICommonService green)
    {
        this.green = green;
    }
 
    // Members that DO something can go here...
}

public ISomeService Main()
{
    return new MyService(
        new BlueService(
            new GreenService()),
        new RedService());
}

In a follow-up to his earlier post on Constructor Over-Injection, Jeffrey Palermo changes his stance on Constructor Over-Injection from anti-pattern to the more palatable code smell. In this post I introduce the concept of a Facade Service and outline a refactoring that addresses this code smell.

If I should extract a core message from Jeffrey Palermo's blog post it would be that it's a code smell if you have a class that takes too many dependencies in its constructor.

I can only agree, but only so far as it's a code smell. However, it has nothing to do with DI in general or Constructor Injection specifically. Rather, it's a smell that indicates a violation of the Single Responsibility Principle (SRP). Let's review the example constructor:

public OrderProcessor(IOrderValidator validator,
                      IOrderShipper shipper,
                      IAccountsReceivable receivable,
                      IRateExchange exchange,
                      IUserContext userContext)

In this version, I even added IOrderShipper back in as I described in my earlier post. Surely, five constructor parameters are too many.

Constructor Injection makes SRP violations glaringly obvious.

What's not to like? My personal threshold lies somewhere around 3-4 constructor parameters, so whenever I hit three, I start to consider if I could perhaps aggregate some of the dependencies into a new type.

I call such a type a Facade Service. It's closely related to Parameter Objects, but the main difference is that a Parameter Object only moves the parameters to a common root, while a Facade Service hides the aggregate behavior behind a new abstraction. While the Facade Service may start its life as a result of a pure mechanistic refactoring, it often turns out that the extracted behavior represents a Domain Concept in its own right. Congratulations: you've just move a little closer to adhering to the SRP!

Let's look at Jeffrey Palermo's OrderProcessor example. The core implementation of the class is reproduced here (recall that in my version, IOrderShipper is also an injected dependency):

public SuccessResult Process(Order order)
{
    bool isValid = _validator.Validate(order);
    if (isValid)
    {
        Collect(order);
        _shipper.Ship(order);
    }
 
    return CreateStatus(isValid);
}
 
private void Collect(Order order)
{
    User user = _userContext.GetCurrentUser();
    Price price = order.GetPrice(_exchange, _userContext);
    _receivable.Collect(user, price);
}

If you examine the code it should quickly become apparent that the Collect method encapsulates a cluster of dependencies: IAccountsReceivable, IRateExchange and IUserContext. In this case it's pretty obvious because they are already encapsulated in a single private method. In real production code, you may need to perform a series of internal refactorings before a pattern starts to emerge and you can extract an interface that aggregates several dependencies.

Now that we have identified the cluster of dependencies, we can extract an interface that closely resembles the Collect method:

public interface IOrderCollector
{
    void Collect(Order order);
}

In lieu of a better name, I simply chose to call it IOrderCollector, but what's interesting about extracting Facade Services is that over time, they often turn out to be previously implicit Domain Concepts that we have now dragged out in the open and made explicit.

We can now inject IOrderCollector into OrderProcessor and change the implementation of the private Collect method:

private void Collect(Order order)
{
    _collector.Collect(order);
}

Next, we can remove the redundant dependencies, leaving us with this constructor:

public OrderProcessor(IOrderValidator validator,
                      IOrderShipper shipper,
                      IOrderCollector collector)

With three constructor parameters it's much more acceptable, but we can always consider repeating the procedure and extract a new Facade Service that aggregates IOrderShipper and IOrderCollector.

The original behavior from the Collect method is still required, but is now implemented in the OrderCollector class:

public class OrderCollector : IOrderCollector
{
    private readonly IUserContext _userContext;
    private readonly IRateExchange _exchange;
    private readonly IAccountsReceivable _receivable;
 
    public OrderCollector(IAccountsReceivable receivable,
                          IRateExchange exchange,
                          IUserContext userContext)
    {
        _receivable = receivable;
        _exchange = exchange;
        _userContext = userContext;
    }
 
    #region IOrderCollector Members
 
    public void Collect(Order order)
    {
        User user = _userContext.GetCurrentUser();
        Price price = 
            order.GetPrice(_exchange, _userContext);
        _receivable.Collect(user, price);
    }
 
    #endregion
}

Here's another class with three constructor parameters, which falls within the reasonable range. However, once again, we can begin to consider whether the interaction between IUserContext and the Order could be better modeled.

In outline form, the Introduce Facade Service refactoring follows these steps:

1. Analyze how dependencies interact to identify clusters of behavior.
2. Extract an interface from these clusters.
3. Copy the original implementation to a class that implements the new interface.
4. Inject the new interface into the consumer.
5. Replace the original implementation with a call the new dependency.
6. Remove the redundant dependencies.
7. Rinse and repeat :)

The beauty of Facade Services is that we can keep wrapping one Facade Service in new Facade Services to define more and more coarse-grained building blocks as we get closer and closer to the application boundary.

Keeping each class and its dependencies to simple interactions also makes it much easier to unit test all of them because none of them do anything particularly complex.

Adhering strictly to Constructor Injection makes it easy to see when one violates the SRP and should refactor to an Facade Service.

Update (2011.04.10): In my book I've changed the name of this concept to Facade Service as it more clearly communicates the relationship with the Facade pattern.

Last modified (2011.08.23): Changed all references to Aggregate Service (the old name of the concept) to Facade Service.

Comments

AutoFixture 1.0 is now available on the CodePlex site! Compared to Release Candidate 2 there are no changes.

The 1.0 release page has more details about this particular release, but essentially this is RC2 promoted to release status.

It's been almost a year since I started development on AutoFixture and I must say that it has been an exciting and fulfilling experience! The API has evolved, but has turned out to be surprisingly flexible, yet robust. I even had some positive surprises along the way as it dawned on me that I could do new fancy things I hadn't originally considered.

If you use the Likeness feature (of which I have yet to write), you will run into this bug in Visual Studio. The bug is only in IntelliSense, so any code using Likeness will compile and work just fine.

While this release marks the end of AutoFixture's initial days, it also marks the beginning of AutoFixture 2.0. I already have lots of plans for making it even more extensible and powerful, as well as plans for utility libraries that integrate with, say, Moq or Rhino Mocks. It's going to be an exciting new voyage!

Comments

In a reaction to Uncle Bob's recent post on Dependency Injection Inversion, Colin Decker writes that he doesn't consider the use of the single Guice @Inject annotation particularly problematic. As I read it, the central argument is that

annotations are not code. By themselves, they do nothing.

I'll have to take that at face value, but if we translate this reasoning to .NET it certainly holds true. Attributes don't do anything by themselves.

I'm not aware of any DI Container for .NET that requires us to sprinkle attributes all over our code to work (I don't consider MEF a DI Container), but for the sake of argument, let's assume that such a container exists (let's call it Needle). Would it be so bad if we had to liberally apply the Needle [Inject] attribute in large parts of our code bases?

Colin suggests no. As usual, my position is that it depends, but in most cases I would consider it bad.

If Needle is implemented like most libraries, InjectAttribute is just one of many types that make up the entire API. Other types would include NeedleContainer and its associated classes.

Java annotations may work differently, but in .NET we need to reference a library to apply one of its attributes. To apply the [Inject] attribute, we would have to reference Needle, and herein lies the problem.

Once Needle is referenced, it becomes much easier for a junior developer to accidentally start directly using other parts of the Needle API. Particularly he or she may start using Needle as a Service Locator. When that happens, Needle is no longer a passive participant of the code, but a very active one, and it becomes much harder to separate the code from the Container.

To paraphrase Uncle Bob: I don't want to write a Needle application.

We can't even protect ourselves from accidental usage by writing a convention-based unit test that fails if Needle is referenced by our code, because it must be referenced for the [Inject] attribute to be applied.

The point is that the attribute drags in a reference to the entire container, which in my opinion is a bad thing.

So when would it be less problematic?

If Needle was implemented in such a way that InjectAttribute was defined in an assembly that only contains that one type, and the rest of Needle was implemented in a different assembly, the attribute wouldn't drag the rest of the container along.

Whether this whole analysis makes sense at all in Java, and whether Guice is implemented like that, I can't say, but in most cases I would consider even a single attribute to be unacceptable pollution of my code base.

Comments