ploeh blog danish software design

This post is the third in a series about Poka-yoke Design - also known as encapsulation.

Automatic properties are one of the most redundant features of C#. I know that some people really love them, but they address a problem you shouldn't have in the first place.

I totally agree that code like this looks redundant:

private string name;
public string Name
{
    get { return this.name; }
    set { this.name = value; }
}

However, the solution is not to write this instead:

public string Name { get; set; }

The problem with the first code snippet isn't that it contains too much ceremony. The problem is that it breaks encapsulation. In fact

"[…] getters and setters do not achieve encapsulation or information hiding: they are a language-legitimized way to violate them."

James O. Coplien & Gertrud Bjørnvig. Lean Architecture. Wiley. 2010. p. 134.

While I personally think that properties do have their uses, I very rarely find use for automatic properties. They are never appropriate for reference types, and only rarely for value types.

Code Smell: Automatic Reference Type Property #

First of all, let's consider the very large set of properties that expose a reference type.

In the case of reference types, null is a possible value. However, when we think about Poka-yoke design, null is never an appropriate value because it leads to NullReferenceExceptions. The Null Object pattern provides a better alternative to deal with situations where a value might be undefined.

In other words, an automatic property like the Name property above is never appropriate. The setter must have some kind of Guard Clause to protect it against null (and possibly other invalid values). Here's the most fundamental example:

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

As an alternative, a Guard Clause could also check for null and provide a default Null Object in the cases where the assigned value is null:

private string name;
public string Name
{
    get { return this.name; }
    set 
    {
        if (value == null)
        {
            this.name = "";
            return;
        }
        this.name = value; 
    }
}

However, this implementation contains a POLA violation because the getter sometimes returns a different value than what was assigned. It's possible to fix this problem by adding an associated boolean field indicating whether the name was assigned null so that null can be returned from the setter in this special case, but that leads to another code smell.

Code Smell: Automatic Value Type Property #

If the type of the property is a value type, the case is less clear-cut because value types can't be null. This means that a Null Guard is never appropriate. However, directly consuming a value type may still be inappropriate. In fact, it's only appropriate if the class can meaningfully accept and handle any value of that type.

If, for example, the class can really only handle a certain subset of all possible values, a Guard Clause must be introduced. Consider this example:

public int RetryCount { get; set; }

This property might be used to set the appropriate number or retries for a given operation. The problem with using an automatic property is that it's possible to assign a negative value to it, and that wouldn't make any sense. One possible remedy is to add a Guard Clause:

private int retryCount;
public int RetryCount
{
    get { return this.retryCount; }
    set
    {
        if (value < 0)
        {
            throw new ArgumentOutOfRangeException();
        }
        this.retryCount = value;
    }
}

However, in many cases, exposing a primitive property is more likely to be a case of Primitive Obsession.

Improved Design: Guard Clause #

As I described above, the most immediate fix for automatic properties is to properly implement the property with a Guard Clause. This ensures that the class' invariants are properly encapsulated.

Improved Design: Value Object Property #

When the automatic property is a value type, a Guard Clause may still be in order. However, when the property is really a symptom of Primitive Obsession, a better alternative is to introduce a proper Value Object.

Consider, as an example, this property:

public int Temperature { get; set; }

This is bad design for a number of reasons. It doesn't communicate the unit of measure and allows unbounded values to be assigned. What happens if - 100 is assigned? If the unit of measure is Celcius it should succeed, although in the case when it's Kelvin, it should fail. No matter the unit of measure, attempting to assign int.MinValue should fail.

A more robust design can be had if we introduce a new Temperature type and change the property to have that type. Apart from protection of invariants it would also encapsulate conversion between different temperature scales.

However, if that Value Object is implemented as a reference type the situation is equivalent to the situation described above, and a Null Guard is necessary. Only in the case where the Value Object is implemented as a value type is an anonymous property appropriate.

The bottom line is that automatic properties are rarely appropriate. In fact, they are only appropriate when the type of the property is a value type and all conceivable values are allowed. Since there are a few cases where automatic properties are appropriate their use can't be entirely dismissed, but it should be treated as warranting further investigation. It's a code smell, not an anti-pattern.

On a different note properties also violate the Law of Demeter, but that's the topic of a future blog post...

Comments

This post is the second in a series about Poka-yoke Design - also known as encapsulation.

Many classes have a tendency to consume or expose primitive values like integers and strings. While such primitive types exist on any platform, they tend to lead to procedural code. Furthermore they often break encapsulation by allowing invalid values to be assigned.

This problem has been addressed many times before. Years ago Jimmy Bogard provided an excellent treatment of the issue, as well as guidance on how to resolve it. In relation to AutoFixture I also touched upon the subject some time ago. As such, the current post is mostly a placeholder.

However, it's worth noting that both Jimmy's and my own post address the concern that strings and integers do not sufficiently encapsulate the concepts of Zip codes and phone numbers.

• When a Zip code is represented as a string it's possible to assign values such as null, string.Emtpy, “foo”, very long strings, etc. Jimmy's ZipCode class encapsulates the concept by guaranteeing that an instance can only be successfully created with a correct value.
• When a Danish phone number is represented as an integer it's possible to assign values such as - 98, 0, int.MaxValue, etc. Once again the DanishPhoneNumber class from the above example encapsulates the concept by guaranteeing that an instance can only be successfully created with a correct value.

Encapsulation is broken unless the concept represented by a primitive value can truly take any of the possible values of the primitive type. This is rarely the case.

Design Smell #

A class consumes a primitive type. However, further analysis shows that not all possible values of the type are legal values.

Improved Design #

Encapsulate the primitive value in a Value Object that contains appropriate Guard Clauses etc. to guarantee that only valid instances are possible.

Primitives tend to not be fail-safe, but encapsulated Value Objects are.

Comments

This post is the first in a series about Poka-yoke Design - also known as encapsulation.

A common problem in API design is temporal coupling, which occurs when there's an implicit relationship between two, or more, members of a class requiring clients to invoke one member before the other. This tightly couples the members in the temporal dimension.

The archetypical example is the use of an Initialize method, although copious other examples can be found - even in the BCL. As an example, this usage of EndpointAddressBuilder compiles, but fails at run-time:

var b = new EndpointAddressBuilder();
var e = b.ToEndpointAddress();

It turns out that at least an URI is required before an EndpointAddress can be created. The following code compiles and succeeds at run-time:

var b = new EndpointAddressBuilder();
b.Uri = new UriBuilder().Uri;
var e = b.ToEndpointAddress();

The API provides no hint that this is necessary, but there's a temporal coupling between the Uri property and the ToEndpointAddress method.

In the rest of the post I will provide a more complete example, as well as a guideline to improve the API towards Poka-yoke Design.

Smell Example #

This example describes a more abstract code smell, exhibited by the Smell class. The public API looks like this:

public class Smell
{
    public void Initialize(string name)
 
    public string Spread()
}

Semantically the name of the Initialize method is obviously a clue, but on a structural level this API gives us no indication of temporal coupling. Thus, code like this compiles, but throws an exception at run-time:

var s = new Smell();
var n = s.Spread();

It turns out that the Spread method throws an InvalidOperationException because the Smell has not been initialized with a name. The problem with the Smell class is that it doesn't properly protect its invariants. In other words, encapsulation is broken.

To fix the issue the Initialize method must be invoked before the Spread method:

var sut = new Smell();
sut.Initialize("Sulphur");
var n = sut.Spread();

While it's possible to write unit tests that explore the behavior of the Smell class, it would be better if the design was improved to enable the compiler to provide feedback.

Improvement: Constructor Injection #

Encapsulation (Poka-yoke style) requires that the class can never be in an inconsistent state. Since the name of the smell is required, a guarantee that it is always available must be built into the class. If no good default value is available, the name must be requested via the constructor:

public class Fragrance : IFragrance
{
    private readonly string name;
 
    public Fragrance(string name)
    {
        if (name == null)
        {
            throw new ArgumentNullException("name");
        }
 
        this.name = name;
    }
 
    public string Spread()
    {
        return this.name;
    }
}

This effectively guarantees that the name is always available in all instances of the class. There  are also positive side effects:

• The cyclomatic complexity of the class has been reduced
• The class is now immutable, and thereby thread-safe

However, there are times when the original version of the class implements an interface that causes the temporal coupling. It might have looked like this:

public interface ISmell
{
    void Initialize(string name);
 
    string Spread();
}

In many cases the injected value (name) is unknown until run-time, in which case straight use of the constructor seems prohibitive - after all, the constructor is an implementation detail and not part of the loosely coupled API. When programming against an interface it's not possible to invoke the constructor.

There's a solution for that as well.

Improvement: Abstract Factory #

To decouple the methods in the ISmell (ha ha) interface the Initialize method can be moved to a new interface. Instead of mutating the (inconsistent) state of a class, the Create method (formerly known as Initialize) returns a new instance of the IFragrance interface:

public interface IFragranceFactory
{
    IFragrance Create(string name);
}

The implementation is straightforward:

public class FragranceFactory : IFragranceFactory
{
    public IFragrance Create(string name)
    {
        if (name == null)
        {
            throw new ArgumentNullException("name");
        }
        return new Fragrance(name);
    }
}

This enables encapsulation because both the FragranceFactory and Fragrance classes protect their invariants. They can never be in an inconsistent state. A client previously interacting with the ISmell interface can use the IFragranceFactory/IFragrance combination to achieve the same funcionality:

var f = factory.Create(name);
var n = f.Spread();

This is better because improper use of the API can now be detected by the compiler instead of at run-time. An interesting side-effect by moving towards a more statically declared interaction structure is that classes tend towards immutability. Immutable classes are automatically thread-safe, which is an increasingly important trait in the (relatively) new multi-core era.

Comments

IFooDevice
{
    Task Initialize(string comPort);
    int DoSomething();
}

class FooDevice : IFooDevice
{
    public FooDevice(string comPort)
    {
        // Initialize(comPort);
    }
    
    private void Initialize(string comPort) { /* ... */ }
}

IFooDeviceFactory
{
    Task<IFooDevice> Create(string comPort);
}

class FooDevice : IFooDevice
{
    private FooDevice() { }

    public static Task<FooDevice> Create(string comPort)
    {
        var device = new FooDevice();
        await device.Initialize(string comPort);

        return device;
    }

    private Task Initialize(string comPort) { /* ... */ }
}

			class SmellFactory : ISmellFactory
			{
				ISmell Create(string name)
				{
					var smell = new Smell();
					smell.Initialize(name);
					return smell;
				}
			}
			

IFooDeviceFactory
{
    Task<IFooDevice> Create(string comPort);
}

IFooDevice
{
    int DoSomething();
}

Encapsulation is one of the most misunderstood aspects of object-oriented programming. Most people seem to think that the related concept of information hiding simply means that private fields should be exposed by public properties (or getter/setter methods in languages that don't have native properties).

Have you ever wondered what's the real benefit to be derived from code like the following?

private string name;
public string Name
{
    get { return this.name; }
    set { this.name = value; }
}

This feels awfully much like redundant code to me (and automatic properties are not the answer - it's just a compiler trick that still creates private backing fields). No information is actually hidden. Derick Bailey has a good piece on why this view of encapsulation is too narrow, so I'm not going to reiterate all his points here.

So then what is encapsulation?

The whole point of object-orientation is to produce cohesive pieces of code (classes) that solve given problems once and for all, so that programmers can use those classes without having to learn about the intricate details of the implementations.

This is what encapsulation is all about: exposing a solution to a problem without requiring the consumer to fully understand the problem domain.

This is what all well-designed classes do.

• You don't have to know the intricate details of TDS to use ADO.NET against SQL Server.
• You don't have to know the intricate details of painting on the screen to use WPF or Windows Forms.
• You don't have to know the intricate details of Reflection to use a DI Container.
• You don't have to know how to efficiently sort a list in order to efficiently sort a list in .NET.
• Etc.

What makes encapsulation so important is exactly this trait. The class must hide the information it encapsulates in order to protect it against ‘naïve' users. Wikipedia has this to say:

Hiding the internals of the object protects its integrity by preventing users from setting the internal data of the component into an invalid or inconsistent state.

Keep in mind that users are expected to not fully understand the internal implementation of a class. This makes it obvious what encapsulation is really about:

Encapsulation is a fail-safe mechanism.

By corollary, encapsulation does not mean hiding complexity. Whenever complexity is hidden (as is the case for Providers) feedback time increases. Rapid feedback is much preferred, so delaying feedback is not desirable if it can be avoided.

Encapsulation is not about hiding complexity, but conversely exposing complexity in a fail-safe manner.

In Lean this is known as Poka-yoke, so I find it only fitting to think about encapsulation as Poka-yoke Design: APIs that make it as hard as possible to do the wrong thing. Considering that compilation is the cheapest feedback mechanism, it's preferable to design APIs so that the code can only compile when classes are used correctly.

In a series of blog posts I will look at various design smells that break encapsulation, as well as provide guidance on how to improve the design to make it safer, thus going from smell to fragrance.

1. Design Smell: Temporal Coupling
2. Design Smell: Primitive Obsession
3. Code Smell: Automatic Property
4. Design Smell: Redundant Required Attribute
5. Design Smell: Default Constructor
6. DI Container smell: Captive Dependency

Postscript: At the Boundaries, Applications are Not Object-Oriented

Comments

Recently I had the inclination to do the Tennis Kata a couple of times. The first time I saw it I thought it wasn't terribly interesting as an exercise in C# development. It would basically just be an application of the State pattern, so I decided to make it a bit more interesting. More or less by intuition I decided to give myself the following constraints:

• All types must be immutable
• All members must have a cyclomatic complexity of 1

Now that's more interesting :)

Given these constraints, what would be the correct approach? Given that this is a finite state machine with a fixed number of states, the Visitor pattern will be a good match.

Each player's score can be modeled as a Value Object that can be one of these types:

• ZeroPoints
• FifteenPoints
• ThirtyPoints
• FortyPoints
• AdvantagePoint
• GamePoint

All of these classes implement the IPoints interface:

public interface IPoints
{
    IPoints Accept(IPoints visitor);
 
    IPoints LoseBall();
 
    IPoints WinBall(IPoints opponentPoints);
 
    IPoints WinBall(AdvantagePoint opponentPoints);
 
    IPoints WinBall(FortyPoints opponentPoints);
}

The interesting insight here is that until the opponent's score reaches FortyPoints nothing special happens. Those states can be effectively collapsed into the WinBall(IPoints) method. However, when the opponent either has FortyPoints or AdvantagePoint, special things happen, so IPoints has specialized methods for those cases. All implementations should use double dispatch to invoke the correct overload of WinBall, so the Accept method must be implemented like this:

public IPoints Accept(IPoints visitor)
{
    return visitor.WinBall(this);
}

That's the core of the Visitor pattern in action. When the implementer of the Accept method is either FortyPoints or AdvantagePoint, the specialized overload will be invoked.

It's now possible to create a context around a pair of IPoints (called a Game) to implement a method to register that Player 1 won a ball:

public Game PlayerOneWinsBall()
{
    var newPlayerOnePoints = this.PlayerTwoScore
        .Accept(this.PlayerOneScore);
    var newPlayerTwoPoints = 
        this.PlayerTwoScore.LoseBall();
    return new Game(
        newPlayerOnePoints, newPlayerTwoPoints);
}

A similar method for player two simply reverses the roles. (I'm currently reading Lean Architecture, but have yet to reach the chapter on DCI. However, considering what I've already read about DCI, this seems to fit the bill pretty well… although I might be wrong on that account.)

The context calculates new scores for both players and returns the result as a new instance of the Game class. This keeps the Game and IPoints implementations immutable.

The new score for the winner depends on the opponent's score, so the appropriate overload of WinBall should be invoked. The Visitor implementation makes it possible to pick the right overload without resorting to casts and if statements. As an example, the FortyPoints class implements the three WinBall overloads like this:

public IPoints WinBall(IPoints opponentPoints)
{
    return new GamePoint();
}
 
public IPoints WinBall(FortyPoints opponentPoints)
{
    return new AdvantagePoint();
}
 
public IPoints WinBall(AdvantagePoint opponentPoints)
{
    return this;
}

It's also important to correctly implement the LoseBall method. In most cases, losing a ball doesn't change the current state of the loser, in which case the implementation looks like this:

public IPoints LoseBall()
{
    return this;
}

However, when the player has advantage and loses the ball, he or she loses the advantage, so for the AdvantagePoint class the implementation looks like this:

public IPoints LoseBall()
{
    return new FortyPoints();
}

To keep things simple I decided to implicitly model deuce as both players having FortyPoints, so there's not explicit Deuce class. Thus, AdvantagePoint returns FortyPoints when losing the ball.

Using the Visitor pattern it's possible to keep the cyclomatic complexity at 1. The code has no branches or loops. It's immutable to boot, so a game might look like this:

[Fact]
public void PlayerOneWinsAfterHardFight()
{
    var game = new Game()
        .PlayerOneWinsBall()
        .PlayerOneWinsBall()
        .PlayerOneWinsBall()
        .PlayerTwoWinsBall()
        .PlayerTwoWinsBall()
        .PlayerTwoWinsBall()
        .PlayerTwoWinsBall()
        .PlayerOneWinsBall()
        .PlayerOneWinsBall()
        .PlayerOneWinsBall();
 
    Assert.Equal(new GamePoint(), game.PlayerOneScore);
    Assert.Equal(new FortyPoints(), game.PlayerTwoScore);
}

In case you'd like to take a closer look at the code I'm attaching it to this post. It was driven completely by using the AutoFixture.Xunit extension, so if you are interested in idiomatic AutoFixture code it's also a good example of that.

Comments

Generics in .NET are wonderful, but sometimes when doing Test-Driven Development against a generic class I've felt frustrated because I've been feeling that dropping down to the lowest common denominator and testing against, say, Foo<object> doesn't properly capture the variability inherent in generics. On the other hand, writing the same test for five different types of T have seemed too wasteful (not to mention boring) to bother with.

That's until it occurred to me that in xUnit.net (and possibly other unit testing frameworks) I can define a generic test class. As an example, I wanted to test-drive a class with this class definition:

public class Interval<T>

Instead of writing a set of tests against Interval<object> I rather wanted to write a set of tests against a representative set of T. This is so easy to do that I don't know why I haven't thought of it before. I simply declared the test class itself as a generic class:

public abstract class IntervalFacts<T>

The reason I declared the class as abstract is because that effectively prevents the test runner from trying to run the test methods directly on the class. That would fail because T is still open. However, it enabled me to write tests like this:

[Theory, AutoCatalogData]
public void MinimumIsCorrect(IComparable<T> first, 
    IComparable<T> second)
{
    var sut = new Interval<T>(first, second);
    IComparable<T> result = sut.Minimum;
    Assert.Equal(result, first);
}

In this test, I also use AutoFixture's xUnit.net extension, but that's completely optional. You might as well just write an old-fashioned unit test, but then you'll need a SUT Factory that can resolve generics. If you don't use AutoFixture any other (auto-mocking) container will do, and if you don't use one of those, you can define a Factory Method that creates appropriate instances of T (and, in this particular case, instances of IComparable<T>).

In the above example I used a [Theory], but I might as well have been using a [Fact] as long as I had a container or a Factory Method to create the appropriate instances.

The above test doesn't execute in itself because the owning class is abstract. I needed to declare an appropriate set of constructed types for which I wanted the test to run. To do that, I defined the following test classes:

public class DecimalIntervalFacts : IntervalFacts<decimal> { }
public class StringIntervalFacts : IntervalFacts<string> { }
public class DateTimeIntervalFacts : IntervalFacts<DateTime> { }
public class TimSpanIntervalFacts : IntervalFacts<TimeSpan> { }

The only thing these classes do is to each pick a particular type for T. However, since they are concrete classes with test methods, the test runner will pick up the test cases and execute them. This means that the single unit test I wrote above is now being executed four times - one for each constructed type.

It's even possible to specialize the generic class and add more methods to a single specialized class. As a sanity check I wanted to write a set of tests specifically against Interval<int>, so I added this class as well:

public class Int32IntervalFacts : IntervalFacts<int>
{
    [Theory]
    [InlineData(0, 0, 0, true)]
    [InlineData(-1, 1, -1, true)]
    [InlineData(-1, 1, 0, true)]
    [InlineData(-1, 1, 1, true)]
    [InlineData(-1, 1, -2, false)]
    [InlineData(-1, 1, 2, false)]
    public void Int32ContainsReturnsCorrectResult(
        int minimum, int maximum, int value,
        bool expectedResult)
    {
        var sut = new Interval<int>(minimum, maximum);
        var result = sut.Contains(value);
        Assert.Equal(expectedResult, result);
    }
 
    // More tests...
}

When added as an extra class in addition to the four ‘empty' concrete classes above, it now causes each generic method to be executed five times, whereas the above unit test is only executed for the Int32IntervalFacts class (on the other hand it's a parameterized test, so the method is actually executed six times).

It's also possible to write parameterized tests in the generic test class itself:

[Theory]
[InlineData(-1, -1, false)]
[InlineData(-1, 0, true)]
[InlineData(-1, 1, true)]
[InlineData(0, -1, false)]
[InlineData(0, 0, true)]
[InlineData(0, 1, true)]
[InlineData(1, -1, false)]
[InlineData(1, 0, false)]
[InlineData(1, 1, false)]
public void ContainsReturnsCorrectResult(
    int minumResult, int maximumResult,
    bool expectedResult)
{
    // Test method body
}

Since this parameterized test in itself has 9 variations and is declared by IntervalFacts<T> which now has 5 constructed implementers, this single test method will be executed 9 x 5 = 45 times!

Not that the number of executed tests in itself is any measure of the quality of the test, but I do appreciate the ability to write generic unit tests against generic types.

Since I announced AutoFixture 2.1 beta 1 no issues have been reported, so I've now promoted the release to the official 2.1 release. This means that if you already downloaded AutoFixture 2.1 beta 1, you already have the 2.1 binaries. If not, you can head over to the release page to get it.

The NuGet packages will soon be available.

Update (2011.5.4): The NuGet packages are now available.

Recently I partook in a Windows Azure migration workshop, helping developers from existing development organizations port their applications to Windows Azure. Once more an old design smell popped up: SQL Server over-utilization. This ought to be old news to anyone with experience designing software on the Wintel stack, but apparently it bears repetition:

Don't put logic in your database. SQL Server should be used only for persistent storage of data.

(Yes: this post is written in 2011…)

Many years ago I heard that role described as a ‘bit bucket' - you put in data and pull it out again, and that's all you do. No fancy stored procedures or functions or triggers.

Why wouldn't we want to use the database if we have one? Scalability is the answer. SQL Server doesn't scale horizontally. You can't add more servers to take the load off a database server (well, some of my old colleagues will argue that this is possible with Oracle, and that may be true, but with SQL Server it's impossible).

Yes, we can jump through hoops like partitioning and splitting the database up into several smaller databases, but it still doesn't give us horizontal scalability. SQL Server is a bottleneck in any system in which it takes part.

How is this relevant to Windows Azure? It's relevant for two important reasons:

• There's an upper size limit on SQL Azure. Currently that size limit is 50 GB, and while it's likely to grow in the future, there's going to be a ceiling for a long time.
• You can't fine tune the hardware for performance. The server runs on virtual hardware.

Development organizations that rely heavily on the database for execution of logic often need expensive hardware and experienced DBAs to squeeze extra performance out of the database servers. Such people know that write-intensive/append-only tables work best with one type of RAID, while read-intensive tables are better hosted on other file groups on different disks with different RAID configurations.

With SQL Azure you can just forget about all that.

The bottom line is that there are fundamental rules for software development that you must follow if you want to be able to successfully migrate to Windows Azure. I previously described an even simpler sanity check you should perform, but after that you should take a good look at your database.

The best solution is if you can completely replace SQL Server with Azure's very scalable storage services, but those come with their own set of challenges.

Rapid feedback is one of the cornerstones of agile development. The faster we can get feedback, the less it costs to correct errors. Unit testing is one of the ways to get feedback, but not the only one. Each way we can get feedback comes with its own advantages and disadvantages.

In my experience there's a tradeoff between the cost of setting up a feedback mechanism and the level of confidence we can get from it. This is quite intuitive to most people, but I've rarely seen it explicitly described. The purpose of this post is to explicitly describe and relate those different mechanisms.

Compilation #

In compiled languages, compilation provides the first level of feedback. I think a lot of people don't think about this as feedback as (for compiled languages) it's a necessary step before the code can be executed.

The level of confidence may not be very high - we tend to laugh at statements like ‘if it compiles, it works'. However, the compiler still catches a lot of mistakes. Think about it: does your code always compile, or do you sometimes get compilation errors? Do you sometimes try to compile your code just to see if it's possible? I certainly do, and that's because the compiler is always available, and it's the first and fastest level of feedback we get.

Code can be designed to take advantage of this verification step. Constructor Injection, for example, ensures that we can't create a new instance of a class without supplying it with its required dependencies.

It's also interesting to note that anecdotal evidence seems to suggest that unit testing is more prevalent for interpreted languages. That makes a lot of sense, because as developers we need feedback as soon as possible, and if there's no compiler we must reach for the next level of feedback mechanism.

Static code analysis #

The idea of static code analysis isn't specific to .NET, but in certain versions of Visual Studio we have Code Analysis (which is also available as FxCop). Static code analysis is a step up from compilation - not only is code checked for syntactical correctness, but it's also checked against a set of known anti-patterns and idioms.

Static code analysis is encapsulated expert knowledge, yet surprisingly few people use it. I think part of the reason for this is because the ratio of time against confidence isn't as compelling as with other feedback mechanism. It takes some time to run the analysis, but like compilation the level of confidence gained from getting a clean result is not very high.

Personally, I use static code analysis once in a while (e.g. before checking in code to the default branch), but not as often as other feedback mechanisms. Still, I think this type of feedback mechanism is under-utilized.

Unit testing #

Running a unit test suite is one of the most efficient ways to gain rapid feedback. The execution time is often comparable to running static code analysis while the level of confidence is much higher.

Obviously a successful unit test execution is no guarantee that the final application works as intended, but it's a much better indicator than the two previous mechanisms. When presented with the choice of using either static code analysis or unit tests, I prefer unit tests every time because the confidence level is much higher.

If you have sufficiently high code coverage I'd say that a successful unit test suite is enough confidence to check in code and let continuous integration take care of the rest.

Keep in mind that continuous integration and other types of automated builds are feedback mechanisms. If you institute rules where people are ‘punished' for checking in code that breaks the build, you effectively disable the automated build as a source of feedback.

Thus, the rest of these feedback mechanisms should be used rarely (if at all) on developer work stations.

Integration testing #

Integration testing comes in several sub-categories. You can integrate multiple classes from within the same library, or integrate multiple libraries while still using Test Doubles instead of out-of-process resources. At this level, the cost/confidence ratio is comparable to unit testing.

Subcutaneous testing #

A more full level of integration testing comes when all resources are integrated, but the tests are still driven by code instead of through the UI. While this gives a degree of confidence that is close to what you can get from a full systems test, the cost of setting up the entire testing environment is much higher. In many cases I consider this the realm of a dedicated testing department, as it's often a full-time job to maintain the suite of automation tools that enable such tests - particularly if we are talking about distributed systems.

System testing #

This is a test of the full system, often driven through the UI and supplemented with manual testing (such as exploratory testing). This provides the highest degree of confidence, but comes with a very high cost. It's often at this level we find formal acceptance tests.

The time before we can get feedback at this level is often considerable. It may take days or even weeks or months.

Understand the cost/confidence ratio #

The purpose of this post has been to talk about different ways we can get feedback when developing software. If we could get instantaneous feedback with guaranteed confidence it would be easy to choose, but as this is not the case we must understand the benefits and drawbacks of each mechanism.

It's important to realize that there are many mechanisms, and that we can combine them in a staggered approach where we start with fast feedback (like the compiler) with a low degree of confidence and then move through successive stages that each provide a higher level of confidence.

Comments

Developers exposed to ASP.NET are likely to be familiar with the so-called Provider pattern. You see it a lot in that part of the BCL: Role Provider, Membership Provider, Profile Provider, etc. Lots of text has already been written about Providers, but the reason I want to add yet another blog post on the topic is because once in a while I get the question on how it relates to Dependency Injection (DI).

Is Provider a proper way to do DI?

No, it has nothing to do with DI, but as it tries to mimic loose coupling I can understand the confusion.

First things first. Let's start with the name. Is it a pattern at all? Regular readers of this blog may get the impression that I'm fond of calling everything and the kitchen sink an anti-pattern. That's not true because I only make that claim when I'm certain I can hold that position, so I'm not going to denounce Provider as an anti-pattern. On the contrary I will make the claim that Provider is not a pattern at all.

A design pattern is not invented - it's discovered as a repeated solution to a commonly recurring problem. Providers, on the other hand, were invented by Microsoft, and I've rarely seen them used outside their original scope. Secondly I'd also dispute that they solve anything.

That aside, however, I want to explain why Provider is bad design:

• It uses the Constrained Construction anti-pattern
• It hides complexity
• It prevents proper lifetime management
• It's not testable

In the rest of this post I will explain each point in detail, but before I do that we need an example to look at. The old OrderProcessor example suffices, but instead of injecting IOrderValidator, IOrderCollector, and IOrderShipper this variation uses Providers to provide instances of the Services:

public SuccessResult Process(Order order)
{
    IOrderValidator validator = 
        ValidatorProvider.Validator;
    bool isValid = validator.Validate(order);
    if (isValid)
    {
        CollectorProvider.Collector.Collect(order);
        ShipperProvider.Shipper.Ship(order);
    }
 
    return this.CreateStatus(isValid);
}

The ValidatorProvider uses the configuration system to create and return an instance of IOrderValidator:

public static IOrderValidator Validator
{
    get 
    {
        var section = 
            OrderValidationConfigurationSection
                .GetSection();
        var typeName = section.ValidatorTypeName;
        var type = Type.GetType(typeName, true);
        var obj = Activator.CreateInstance(type);
        return (IOrderValidator)obj;
    }
}

There are lots of details I omitted here. I could have saved the reference for later use instead of creating a new instance each time the property is accessed. In that case I would also have had to make the code thread-safe, so I decided to skip that complexity. The code could also be more defensive, but I'm sure you get the picture.

The type name is defined in the app.config file like this:

<orderValidation 
  type="Ploeh.Samples.OrderModel.UnitTest.TrueOrderValidator,
        Ploeh.Samples.OrderModel.UnitTest" />

Obviously, CollectorProvider and ShipperProvider follow the same… blueprint.

This should be well-known to most .NET developers, so what's wrong with this model?

Constrained Construction #

In my book's chapter on DI anti-patterns I describe the Constrained Construction anti-pattern. Basically it occurs every time there's an implicit constraint on the constructor of an implementer. In the case of Providers the constraint is that each implementer must have a default constructor. In the example the culprit is this line of code:

var obj = Activator.CreateInstance(type);

This constrains any implementation of IOrderValidator to have a default constructor, which obviously means that the most fundamental DI pattern Constructor Injection is out of the question.

Variations of the Provider idiom is to supply an Initialize method with a context, but this creates a temporal coupling while still not enabling us to inject arbitrary Services into our implementations. I'm not going to repeat six pages of detailed description of Constrained Construction here, but the bottom line is that you can't fix it - you have to refactor towards true DI - preferably Constructor Injection.

Hidden complexity #

Providers hide the complexity of their implementations. This is not the same as encapsulation. Rather it's a dishonest API and the problem is that it just postpones the moment when you discover how complex the implementation really is.

When you implement a client and use code like the following everything looks deceptively simple:

IOrderValidator validator = 
    ValidatorProvider.Validator;

However, if this is the only line of code you write it will fail, but you will not notice until run-time. Check back to the implementation of the Validator property if you need to refresh the implementation: there's a lot of things that can go wrong here:

• The appropriate configuration section is not available in the app.config file.
• The ValidatorTypeName is not provided, or is null, or is malformed.
• The ValidatorTypeName is correctly formed, but the type in question cannot be located by Fusion.
• The Type doesn't have a default constructor. This is one of the other problems of Constrained Construction: it can't be statically enforced because a constructor is not part of an abstraction's API.
• The created type doesn't implement IOrderValidator.

I'm sure I even forgot a thing or two, but the above list is sufficient for me. None of these problems are caught by the compiler, so you don't discover these issues until you run an integration test. So much for rapid feedback.

I don't like APIs that lie about their complexity.

Hiding complexity does not make an API easier to use; it makes it harder.

An API that hides necessary complexity makes it impossible to discover problems at compile time. It simply creates more friction.

Lifetime management issues #

A Provider exerts too much control over the instances it creates. This is a variation of the Control Freak anti-pattern (also from my book). In the current implementation the Validator property totally violates the Principle of least surprise since it returns a new instance every time you invoke the getter. I did this to keep the implementation simple (this is, after all, example code), but a more normal implementation would reuse the same instance every time.

However, reusing the same instance every time may be problematic in a multi-threaded context (such as a web application) because you'll need to make sure that the implementation is thread-safe. Often, we'd much prefer to scope the lifetime of the Service to each HTTP request.

HTTP request scoping can be built into the Provider, but then it would only work in web applications. That's not very flexible.

What's even more problematic is that once we move away from the Singleton lifestyle (not to be confused with the Singleton design pattern) we may have a memory leak at hand, since the implementation may implement IDisposable. This can be solved by adding a Release method to each Provider, but now we are moving so far into DI Container territory that I find it far more reasonable to just use proper DI instead of trying to reinvent the wheel.

Furthermore, the fact that each Provider owns the lifetime of the Service it controls makes it impossible to share resources. What if the implementation we want to use implements several Role Interfaces each served up by a different Provider? We might want to use that common implementation to share or coordinate state across different Services, but that's not possible because we can't share an instance across multiple providers.

Even if we configure all Providers with the same concrete class, each will instantiate and serve its own separate instance.

Testability #

The Control Freak also impacts testability. Since a Provider creates instances of interfaces based on XML configuration and Activator.CreateInstance, there's no way to inject a dynamic mock.

It is possible to use hard-coded Test Doubles such as Stubs or Fakes because we can configure the XML with their type names, but even a Spy is problematic because we'll rarely have an object reference to the Test Double.

In short, the Provider idiom is not a good approach to loose coupling. Although Microsoft uses it in some of their products, it only leads to problems, so there's no reason to mimic it. Instead, use Constructor Injection to create loosely coupled components and wire them in the application's Composition Root using the Register Resolve Release pattern.

Comments

Implementor < Service < Provider (or ServiceProvider)

Model < StorageService < MySQLProvider