ploeh blog danish software design

My recent series of blog posts about Poka-yoke Design generated a few responses (I would have been disappointed had this not been the case). Quite a few of these reactions relate to various serialization or translation technologies usually employed at application boundaries: Serialization, XML (de)hydration, UI validation, etc. Note that such translation happens not only at the perimeter of the application, but also at the persistence layer. ORMs are also a translation mechanism.

Common to most of the comments is that lots of serialization technologies require the presence of a default constructor. As an example, the XmlSerializer requires a default constructor and public writable properties. Most ORMs I've investigated seem to have the same kind of requirements. Windows Forms and WPF Controls (UI is also an application boundary) also must have default constructors. Doesn't that break encapsulation? Yes and no.

Objects at the Boundary #

It certainly would break encapsulation if you were to expose your (domain) objects directly at the boundary. Consider a simple XML document like this one:

<name>
  <firstName>Mark</firstName>
  <lastName>Seemann</lastName>
</name>

Whether or not we have formal a contract (XSD), we might stipulate that both the firstName and lastName elements are required. However, despite such a contract, I can easily create a document that breaks it:

<name>
  <firstName>Mark</firstName>
</name>

We can't enforce the contract as there's no compilation step involved. We can validate input (and output), but that's a different matter. Exactly because there's no enforcement it's very easy to create malformed input. The same argument can be made for UI input forms and any sort of serialized byte sequence. This is why we must treat all input as suspect.

This isn't a new observation at all. In Patterns of Enterprise Application Architecture, Martin Fowler described this as a Data Transfer Object (DTO). However, despite the name we should realize that DTOs are not really objects at all. This is nothing new either. Back in 2004 Don Box formulated the Four Tenets of Service Orientation. (Yes, I know that they are not in vogue any more and that people wanted to retire them, but some of them still make tons of sense.) Particularly the third tenet is germane to this particular discussion:

Services share schema and contract, not class.

Yes, and that means they are not objects. A DTO is a representation of such a piece of data mapped into an object-oriented language. That still doesn't make them objects in the sense of encapsulation. It would be impossible. Since all input is suspect, we can hardly enforce any invariants at all.

Often, as Craig Stuntz points out in a comment to one of my previous posts, even if the input is invalid, we want to capture what we did receive in order to present a proper error message (this argument also applies on machine-to-machine boundaries). This means that any DTO must have very weak invariants (if any at all).

DTOs don't break encapsulation because they aren't objects at all.

Don't be fooled by your tooling. The .NET framework very, very much wants you to treat DTOs as objects. Code generation ensues.

However, the strong typing provided by such auto-generated classes gives a false sense of security. You may think that you get rapid feedback from the compiler, but there are many possible ways you can get run-time errors (most notably when you forget to update the auto-generated code based on new schema versions).

An even more problematic result of representing input and output as objects is that it tricks lots of developers into dealing with them as though they represent the real object model. The result is invariably an anemic domain model.

More and more, this line of reasoning is leading me towards the conclusion that the DTO mental model that we have gotten used to over the last ten years is a dead end.

What Should Happen at the Boundary #

Given that we write object-oriented code and that data at the boundary is anything but object-oriented, how do we deal with it?

One option is to stick with what we already have. To bridge the gap we must then develop translation layers that can translate the DTOs to properly encapsulated domain objects. This is the route I take with the samples in my book. However, this is a solution that more and more I'm beginning to think may not be the best. It has issues with maintainability. (Incidentally, that's the problem with writing a book: at the time you're done, you know so much more than you did when you started out… Not that I'm denouncing the book - it's just not perfect…)

Another option is to stop treating data as objects and start treating it as the structured data that it really is. It would be really nice if our programming language had a separate concept of structured data… Interestingly, while C# has nothing of the kind, F# has tons of ways to model data structures without behavior. Perhaps that's a more honest approach to dealing with data… I will need to experiment more with this…

A third option is to look towards dynamic types. In his article Cutting Edge: Expando Objects in C# 4.0, Dino Esposito outlines a dynamic approach towards consuming structured data that shortcuts auto-generated code and provides a lightweight API to structured data. This also looks like a promising approach… It doesn't provide compile-time feedback, but that's only a false sense of security anyway. We must resort to unit tests to get rapid feedback, but we're all using TDD already, right?

In summary, my entire series about encapsulation relates to object-oriented programming. Although there are lots of technologies available to represent boundary data as ‘objects', they are false objects. Even if we use an object-oriented language at the boundary, the code has nothing to do with object orientation. Thus, the Poka-yoke Design rules don't apply there.

Now go back and reread this post, but replace ‘DTO' with ‘Entity' (or whatever your ORM calls its representation of a relational table row) and you should begin to see the contours of why ORMs are problematic.

P.S. 2022-05-02. See also At the boundaries, applications aren't functional.

Comments

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

Default constructors are code smells. There you have it. That probably sounds outrageous, but consider this: object-orientation is about encapsulating behavior and data into cohesive pieces of code (classes). Encapsulation means that the class should protect the integrity of the data it encapsulates. When data is required, it must often be supplied through a constructor. Conversely, a default constructor implies that no external data is required. That's a rather weak statement about the invariants of the class.

Please be aware that this post represents a smell. This indicates that whenever a certain idiom or pattern (in this case a default constructor) is encountered in code it should trigger further investigation.

As I will outline below, there are several scenarios where default constructors are perfectly fine, so the purpose of this blog post is not to thunder against default constructors. It's to provide food for thought.

If you have read my book you will know that Constructor Injection is the dominating DI pattern exactly because it statically advertises dependencies and protects the integrity of those dependencies by guaranteeing that an initialized consumer is always in a consistent state. This is fail-safe design because the compiler can enforce the relationship, thus providing rapid feedback.

This principle extends far beyond DI. In a previous post I described how a constructor with arguments statically advertises that the argument is required:

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;
    }
}

The Fragrance class protects the integrity of the name by requiring it through the constructor. Since this class requires the name to implement its behavior, requesting it through the constructor is the correct thing to do. A default constructor would not have been fail-safe, since it would introduce a temporal coupling.

Consider that objects are supposed to be containers of behavior and data. Whenever an object contains data, the data must be encapsulated. In the (very common) case where no meaningful default value can be defined, the data must be provided via the constructor. Thus, default constructors might indicate that encapsulation is broken.

When are Default Constructors OK? #

There are still scenarios where default constructors are in order (I'm sure there are more than those listed here).

• If a default constructor can assign meaningful default values to all contained fields a default constructor still protects the invariants of the class. As an example, the default constructor of UriBuilder initializes its internal values to a consistent set that will build the Uri http://localhost unless one or more of its properties are subsequently manipulated. You may agree or disagree with this default behavior, but it's consistent and so encapsulation is preserved.
• If a class contains no data obviously there is no data to protect. This may be a symptom of the Feature Envy code smell, which is often evidenced by the class in question being a concrete class.
  • If such a class can be turned into a static class it's a certain sign of Feature Envy.
  • If, on the other hand, the class implements an interface, it might be a sign that it actually represents pure behavior.

A class that represents pure behavior by implementing an interface is not necessarily a bad thing. This can be a very powerful construct.

In summary, a default constructor should be a signal to stop and think about the invariants of the class in question. Does the default constructor sufficiently guarantee the integrity of the encapsulated data? If so, the default constructor is appropriate, but otherwise it's not. In my experience, default constructors tend to be the exception rather than the rule.

Comments

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

Recently I saw this apparently enthusiastic tweet reporting from some Microsoft technology event:

[Required] attribute in code automatically creates a non-nullable entry in DB and validation in the webpage - nice […]

I imagine that it must look something like this:

public class Smell
{
    [Required]
    public int Id { get; set; }
}

Every time I see something like this I die a little inside. If you already read my previous posts it should by now be painfully clear why this breaks encapsulation. Despite the [Required] attribute there's no guarantee that the Id property will ever be assigned a value. The attribute is just a piece of garbage making a claim it can't back up.

Code like that is not fail-safe.

I understand that the attribute mentioned in the above tweet is intended to signal to some tool (probably EF) that the property must be mapped to a database schema as non-nullable, but it's still redundant. Attributes are not the correct way to make a statement about invariants.

Improved Design #

The [Required] attribute is redundant because there's a much better way to state that a piece of data is required. This has been possible since .NET 1.0. Here's the Poka-yoke version of that same statement:

public class Fragrance
{
    private readonly int id;
 
    public Fragrance(int id)
    {
        this.id = id;
    }
 
    public int Id
    {
        get { return this.id; }
    }
}

This simple structural design ensures that the ID truly is required (and if the ID can only be positive a Guard Clause can be added). An instance of Fragrance can only be created with an ID. Since this is a structural construction, the compiler can enforce the requirement, giving us rapid feedback.

I do realize that the [Required] attribute mentioned above is intended to address the challenge of mapping objects to relational data and rendering, but instead of closing the impedance mismatch gap, it widens it. Instead of introducing yet another redundant attribute the team should have made their tool understand simple idioms for encapsulation like the one above.

This isn't at all hard to do. As an example, DI Containers thrive on structural information encoded into constructors (this is called Auto-wiring). The team behind the [Required] attribute could have done that as well. The [Required] attribute is a primitive and toxic hack.

This is the major reason I never expect to use EF. It forces developers to break encapsulation, which is a principle upon which I refuse to compromise.

Comments

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.