Special case pattern, primitive obsession and accidental design

I define accidental design as a design that’s present in a software system, but was not consciously chosen for. Instead, it’s driven by other factors, such as tooling, co-workers, past experience, existing code and a whole lot of other reasons. Two of the most common cases I encounter have to do with the special case pattern and primitive obsession.

For this blog post I’ll use examples from the Payroll Case Study.

Special case pattern

The special case pattern is all about null checks. Everybody’s written code like this:

The problem with this code is that I have no clue about the semantic meaning of the if statement. It could be that that an Employee with the specified Id wasn’t found, or it could mean that it was found but just isn’t active anymore, or both. The special case pattern is a way to make your intent more explicit:

It’s more clear what we’re checking here: whether the employee was found or not. Personally, I prefer to decouple the Employee interface from having to know about NotFound states and such, so I’d have GetEmployee return an GetEmployeeResult:

Anyway, what does this have to with accidental design? Well, the entire reason we can check for null is because we’ve defined Employee as a reference type. That means a variable of that type can actually be null. Had it been a struct, or had the language not supported null for reference types, we would have definitely designed it in a different, probably more explicit, way. For example, let’s say we have a method printing the Maximum value for a set of ints, or “Not available” if it’s empty. I’m pretty sure nobody would ever implement that as follows:

if(maxValue == 0) {
return “Not available”;
}
return maxValue.ToString();
}
</pre>
<p>

Instead, we would probably design something to handle the special case of an empty set result explicitly, be it by an IntResult, use a TryGetMax method or making the returned int nullable. The latter might seem contradictory, but the point is that our code makes explicit that we will not always be able to get a maximum value from an int set. In this case we have actually thought about representing the real range of results, whereas by just defaulting to return a null whenever an edge case occurs, we haven’t. The latter situation is accidental design since we did actually make a design decision, but not consciously.

Primitive obsession

Primitive obsession is the tendency to express most of your code in terms of language primitives such as int, float, double, decimal, string, DateTime, Timespan, etc. There are 3 common sources of problems with this approach, nicely illustrated by the following code from the payroll case study:

How often have you introduced bugs because you mixed up the order of the parameters, and accidentally mixed up the salary and the commission rate? I can say I have. Also, what is the commission rate expressed in, is it a percentage or fraction?  What if we want to change the representation of the percentage commission rate to an int? We’ll have to track and change it everywhere.

The underlying problem is that we’re working at the wrong level of abstraction for this problem. ints, decimals, strings, etc are at the level of abstraction of the language/platform, but we’re working at the level of the business here. Things would be much better if we’d change it to something like this:

This way, type safety would prevent us from mixing up the parameters, we can create nice explicit factory methods on our Salary and CommissionRate classes, and changes in data representation would be local to those objects only. Besides that, aren’t we told in high school that an number without a unit is meaningless?

Again, this is accidental design at work: our environment provides us with these primitive types, so we tend to use them. These primitive types, however, are not designed by you. Had they been different, your design would’ve been much different. How much of that would you be comfortable with?

One more example of this is the lack of a Date type in .NET. Because of this we tend to represent all dates as DateTime’s. This has caused me headaches in more than one project, and could’ve been mitigated had we created a Date type ourselves (it’s even worse when you deal with Unix timestamps).

Conclusion

Accidental design might sound like a bad thing in general, but to be honest I actually think a lot of design “decisions” are made that way. If it wasn’t, we’d probably never ship. Still, I think it’s good practice to be aware of as many of them as you can, and know your options when you actually do need to make something more explicit. Other areas where I see a lot of accidental design (or architecture) are choices for languages, frameworks and database technology. I might write about those in a later post.

With regard to the examples: In the case of GetEmployee method, it might actually be a pretty good convention to return null when the specified Employee couldn’t be found. But by being a convention, it’s actually become a conscious decision. Also, an argument I’ve heard against wrapping every primitive is that it introduces needless complexity and a reduced overview of your project due to the proliferation of classes. Those are sound arguments, but in my experience they don’t weigh up against the benefits.