My beef with the open/closed principle

In a previous post I talked about why it’s important to develop a poppycock detector. It can help you to determine when you have to question, investigate or think about things you read or hear. In that post I used this tweet as an example:

In an ideal system, we incorporate new features by extending the system, not by making modifications to existing code. —Uncle Bob

I tried to explain why it can be dangerous to accept —and repeat— statements like that without giving them further thought. In this post I will go into more detail about what it is I don’t like about this particular statement.

The open/closed principe

Let’s start with the definition of the open/closed principle:

In object-oriented programming, the open/closed principle states “software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification”; that is, such an entity can allow its behaviour to be extended without modifying its source code. —Wikipedia

Why I don’t like it

I’m convinced this principle is the cause of a lot overly generic and hard-to-read code covered with layers of indirection. Why? Because a lot of people seem to believe that all code should always adhere to the open/closed principle. Maybe it’s a stage where all developers find themselves in at a specific moment in their career? I know I’ve been there.

The problem is that developers with an open/closed fixation try to anticipate every possible change or extension to the code. This is bad for a number of reasons.

It’s costly

You spend time (and money) up front for things you may never need. This can be seen as a double loss, because the time you’re investing in things you’ll never need isn’t being spent building things your customers do need. Sure, you may save some time later on when it turns out you correctly predicted the future and you do need to implement a change you anticipated, but those occasions are rare.

You can’t predict the future

When writing code it’s often pretty easy to come up with ways in which that code or the system will have to be changed in the future. But it’s impossible to come up with every possible change. Why would you accommodate for possible changes that may never need to happen when you’re not doing the same for other changes that may be more likely to be required in the future?

Note that I’m not talking about changes you will need to do in the next sprint or something like that. If you are absolutely sure you need to change or add something, please do design with that in mind.

Your code becomes overly generic and hard to read

Buy you are a stubborn developer. You take pride in your work, it is your duty to write good code and good code adheres to the holy open/closed principle in every possible way because that’s what it says on your t-shirt.

You try to think of all the ways in which the system might need to change in the future and you make sure that those changes will not require any modifications to the code you’re writing now. You crown yourself Khaleesi of the Great Generic Sea, Mother of DRYgons and who cares if your code is hard to read? No one will have to change it anyway because you successfully predicted the future!

You make it hard on your future self

You are very pleased with yourself because you have designed your code in such a way that features Foo and Bar will be super easy to implement. You look at your code and the elegance brings a tear to your eye.

Two years later there’s still no need for Foo or Bar, but your customers would be thrilled to get feature Baz. No problem. You dive into the old code thinking it will be a breeze to add Baz. What’s this? How does this work? Where is this part implemented? Which idiot designed this code? Time for a git blame! Let’s see, ah, here’s the commit:

Add elegant way to extend the system :’-) —You

The code that made your past self almost emotional is making things quite hard for your current self, bringing up feelings from the opposite side of the emotional spectrum. All the work you did to make things extensible turns out to be a bit one-sided. All the useless work you did with extensibility targeted at Foo and Bar in mind is even making it harder than it should be to implement Baz.

What should you do instead?

The open/closed principle isn’t something you should actively work on. It’s something you will automatically get a more sensible version of when you write Good Code™. If you follow YAGNI, separate the concerns, separate implementation from intent and write tests from the beginning, your code will automatically become easier to extend. You will have to make modifications to code in the future, but that’s not a problem. By following the three simple rules I mentioned you will get code that’s easier to maintain and extend without getting in your way than you when you blindly follow and actively pursue the open/closed principle.

For another take on this, I can highly recommend Rob Ashton’s excellent blog post My relationship with SOLID - The big O.

Develop your poppycock detector

If there’s one thing I have learned from 15 years of programming and over a year of playing Avalon, it’s to trust my poppycock detector. I recently came across this tweet that set of my alarms:

In an ideal system, we incorporate new features by extending the system, not by making modifications to existing code. –Uncle Bob

I looked up the quote on Google but I couldn’t find anything useful besides a link to that same tweet. So I don’t know if Uncle Bob actually said something like it or not, but it doesn’t really matter. Given the lack of fact checking people do, most people will probably believe Uncle Bob did say it. Since Uncle Bob is such an authority in the development community, a lot people will probably accept that statement without giving it further thought.

While I somewhat agree with the quote, I think it is only true within a certain context; a context completely missing from that tweet (hurray for 140 characters). Without the correct context that quote is just plain old poppycock.

So here’s what we have:

• a statement —apparently— by Uncle Bob;
• a statement that’s worthless without proper context.

If you’re relatively new to programming you probably:

• have heard of Uncle Bob and know he’s a man held in high regard;
• lack the experience to detect missing context.

That’s a dangerous combination!

I’ll explain my issues with the quote itself in another post. For now I would like to urge you all to develop your poppycock detector. Don’t accept anything from anyone without thinking about it. If you don’t agree with something being said, speak up. Talk to someone about it. Ask questions. The worst that can happen is that you learn something.

Code should communicate intent

In a previous post I advocated the principle of designing for readability. One of the ways you can achieve a higher degree of readability in your code is making sure it clearly shows what it is supposed to do. If your code communicates intent, it will be much easier for others to grasp the general idea by glancing over it.

An example please

I have been meaning to write about this for a while, but I couldn’t think of a good example. That’s one of the problems you face when you want to illustrate something with code. You don’t want the example to be overly abstract by limiting it to Foo’s and Bar’s, but at the same time, you want it to be understandable without having to provide too much context.

When I received a ‘Nice answer’ badge earlier this week for an answer I posted on Stack Overflow 5 years ago, I stumbled on a great example I wrote myself. What a happy coincidence!

The person asking the question had a simple class that looked like this:

public class MyObject
{
    public int ObjectID { get; set; }
    public string Prop1 { get; set; }
}

The question was how you could remove duplicates from a list of MyObject instances. Two instances were considered to be a duplicate of each other if they had the same value for the ObjectID property.

The answer that got accepted as the correct one suggested this solution:

var distinctList = myList.GroupBy(x => x.ObjectID)
                         .Select(g => g.First())
                         .ToList();

What this does is it groups the list based on the ObjectID and creates a new list with first instance from every group. You probably have to read the entire code block before you realise what it is doing.

My solution was to let the MyObject class implement the IEquatable<T> interface so you can use LINQ’s built-in Distinct method.

public class MyObject : IEquatable<MyObject>
{
    public int ObjectID { get; set; }
    public string Prop1 { get; set; }

    public bool Equals(MyObject other)
    {
        if (other == null) return false;
        else return this.ObjectID.Equals(other.ObjectID); 
    }

    public override int GetHashCode()
    {
        return this.ObjectID.GetHashCode();
    }
}

Just by looking at this class you know that the ObjectID property defines the identity of the class’s instances and that it’s used when checking for equality. This can now be used by LINQ’s Distinct.

var myObjects = new List<MyObject>();
myObjects.Add(new MyObject { ObjectID = 1, Prop1 = "Something" });
myObjects.Add(new MyObject { ObjectID = 2, Prop1 = "Another thing" });
myObjects.Add(new MyObject { ObjectID = 3, Prop1 = "Yet another thing" });
myObjects.Add(new MyObject { ObjectID = 1, Prop1 = "Something" });

var distinctObjects = myObjects.Distinct();

Even though my solution requires more code, I believe it is a lot better because of the intents it communicates:

1. When you look at the class you know how instances will be compared for equality;
2. The use of Distinct clearly show that you want to remove duplicates.

Some pointers

There a quite a few ways you can make the intent of your code more obvious.

Good naming

Use clear and descriptive names. This counts for everything: classes, variables, properties, methods, functions, … Don’t try to come up with shorter names just to save a few keystrokes. Something like numberOfAttendees is longer but far more readable than attndCnt. Why do developers seem to have a tendency to leave out vowels? What’s up with that?

Split things up

If it’s hard to come up with a clear and descriptive name for a class, a function or a method, that might be a sign that you’re trying to do too much in that part of the code. When you split things up it will be easier to name them and if you give them good names people can get away with only reading the names and skipping the implementation.

/* Hard to read */
var myString = "The quick brown fox jumps over the lazy dog.";
var vwls = "aeiou";
var tmpList = new List<Char>();
foreach(var c in myString)
{
    if (!vwls.Contains(c))
    {
        tempList.Add(c);
    }
}
var myString2 = new string(tempList.ToArray());

/* Easier to read */
var sentenceWithoutVowels = RemoveVowels("The quick brown fox jumps over the lazy dog.");

I know there are better ways to remove vowels from a string, but I wanted to show some bad code that’s way too long and confusing on purpose. Imagine the better implementation is inside RemoveVowels. You don’t really need to know how it’s implemented when glancing over code. You know that that function removes vowels and that’s more than enough at that point.

Use built-in features

If you’re trying to do something that’s built into the language, framework or library you’re using, don’t reinvent the wheel. Use built-in features not only for performance reasons, but for recognisability as well. If people reading your code are familiar with the language, framework or library they’ll have an easier time understanding your code if they see things they recognise and know.

Wrapping things up

Making sure your code communicates intent is one of the ways you can design for readability. Naming things is one of the key concepts and it can be surprisingly hard sometimes. Let’s end with a famous and relevant computer science joke.

There are only two hard things in computer science: naming things, cache invalidation and off-by-one errors.

The daily stand-up is not dead

The other day, Jan, one of my awesome colleagues, wrote an interesting blog post on his vision of daily stand-ups. While I agree with him for the most part, there are a few things I have a slightly different opinion about. I also think the way we do stand-ups in the team I’m in is worth sharing. I wanted to reply with a comment to Jan’s blog post, but because of the finite amount of keystrokes I have left, I thought a blog post would be better than a comment.

The same time each day

First off, the same time each day thing. Every team that starts doing stand-ups does so at a designated time. But not everybody starts at the same time, neither might they be at the same location. People that start early will have to do a context switch and thus most likely lose productivity (between you and me, they will). If you need to be present from a distant location, this can also have an impact on the productivity of the stand-up, even in 2016. Technology is a bitch.

Our development team (Jan is in the app team, which is a separate team) is split up into three subteams: Ministry of Code, Fellowship of Code and Cabinet of Code. Every subteam has its own separate stand-up and they start at the same time every day. The Cabinet starts at 9:50, the Fellowship at 10:00 and the Ministry starts at 10:10.

The big advantage of starting at the same time is predictability, which makes it easier if you want to be a silent observer at another team’s stand-up. If we at the Ministry are working on a feature that touches code that someone from another team has written in the past, that person can set a reminder and join our stand-up to see if there’s something they can assist with. Or, if I know that I’ll need to build on a feature currently in development at the Fellowship, I can join their stand-up to learn how things are going and what decisions are being made.

I don’t believe the context switch has a negative impact on productivity, exactly because we always start at the same time. This way you can schedule your work a bit around the stand-up. Besides, in an office where NERF darts are almost eligible for frequent-flyer air miles, you learn to recover quickly from context switches.

Development teams only

Another annoying thing, but not directly related to this, is what is sometimes defined as a team that does a stand-up. Some companies tend to have stand-ups for development teams only. I don’t really see the point if you are closely working together on projects. You should know what’s going on, at least I do. If you have no idea what the guy next to you is doing, you have other problems. Stand-ups should be done with all those involved in a project. Talking to each other about code related issues is easy, the problems usually occur because of unclear or changing requirements.

The number 1 reason to go down the development-team-only road usually is that we assume the business side of things does not have time to participate or that they don’t care.

The Ministry, Fellowship and Cabinet stand-ups are all developer-only stand-ups, although everyone is free to join as a silent observer. A stand-up is basically a mini-meeting. That’s why I like to handle stand-ups the way all meetings should be handled. Only people that actually need to be present should be present. There’s no reason to have project leads, business leads, marketing members, copy members, customer care members and operations members present in every stand-up. If you need the entire project team, that’s probably a sign the scope isn’t suited for a stand-up. Book a separate meeting with a predefined agenda and a fixed start and end time. This will give everyone involved time to prepare which will make things more efficient.

The way we do stand-ups

We started using a Kanban system in our development team a few months ago. After a few weeks we already started seeing benefits, which might be food for another blog post. Before we were using Kanban, our stand-ups were like Jan described them in his post: 15 people in a big circle and everyone got one minute to talk. Someone held up a timer so you could see your time ticking away. It even beeped at 30, 50, 55 and 60 seconds in. That really created some sort of pressure because when your minute was up, you had to stop talking, mid-sentence or not. As a result of the timer and the size of circle, you typically heard things like “Yesterday I worked on feature X, today they same”. Not really valuable information.

Use the board Luke

That all changed when we started using the Kanban system. The most important thing is our Kanban board. Everything starts from the board, even our stand-ups. Instead of talking about what we did yesterday and what we’re going to do today, we talk about the state of the board. We go over each column, start downstream and work our way upstream.

• Any blockers in ‘To launch’? No? Good! Yes? How could we help Operations? Let’s go check after the stand-up.
• Hmm, that ticket has beens stuck in ‘Testing’ for four days now. Let’s go check with QA after the stand-up to see what causing them problems.
• Is ‘Ready for Testing’ full? Yes it is. Is there anything further upstream that could move there but can’t because the WIP limit is reached? Ok, no problem, we’ll take that up with QA as well.
• Anything waiting for a code review? Ok, I’ll handle that one.

When we get to the ‘In progress’ column (which means ‘In progress’ from a development viewpoint) we briefly go over what everyone is working on, check if the estimated due date is still realistic and discuss blockers or issues. If there are blockers or other issues, we take them up with the relevant person after the stand-up.

Finally we also have a glance at the ‘Todo’ column to see if there’s anything that’s more urgent than what we’re working on now.

Being late for a stand-up or working remote

If you can’t make it to the stand-up, that’s not really an issue since everything you need to know is perfectly visible on the board. We don’t have fulltime remote workers, so we only need to cover for people working from home now and then. And, the same applies there, actually. The board tells you what you need to know and if necessary you can always give the rest of the team a short update on Slack.

Not including people from other teams

This isn’t causing us problems either because we know from the board and from our stand-up when to go to other project members. Besides that we also keep the project leads up to date in an informal way. That information ends up in more formal planning meetings and from there it trickles down to the relevant teams and/or team members. This isn’t perfect yet, but it seems to be working pretty OK for now.

Conclusion

I do agree with the general sentiment of Jan’s blog post, but using Kanban and changing our stand-up accordingly has really been an eye-opener for me and changed the way I think about stand-ups. Our stand-ups have decreased in time and increased in efficiency. That’s always a nice combination!

Oh, if you wan’t become a member of the Ministry, the Fellowship or the Cabinet, have a look at our jobs page. We’re always on the lookout for new talent!

Design for readability

Surprisingly enough, one of the hardest things to do when you’re writing code is to keep things simple. There are many causes why we often make code more complicated than it should be:

• Reducing line count. Even when the “gain” is small we tend to focus on this. It’s hard to resist turning three lines of code into one line.
• Trying to be clever. Why keep things simple if you can prove you understand lambdas, monads, recursion, reflection and parallelism? Your colleagues will be so impressed!
• Frameworkitis. You can’t resist the urge to try out the latest JavaScript framework, even though what you’re doing is far to simple to justify the use of it.
• Design pattern hypochondriasis. An unnecessary concern about missing out on design patterns. You think your code resembles something that’s covered by a certain a design pattern and start refactoring your code in order to shoehorn in the pattern.

I try to be conscious of this whenever I’m coding. When you’re writing “clever code”, the short term high it gives you, makes you forget about the long term consequences. Unnecessarily complex code that you haven’t written yourself is hard to grasp. I’ve often had a feeling that I got the general gist of it, but I felt like I was missing something deeper, on a fundamental level. Why is this code so complex? What am I missing here? I’ve even had that happen with code I had written myself only months before.

Try to keep things simple when writing code. That doesn’t mean you don’t have to think about what you’re writing though! Like I said, writing simple code isn’t always easier than writing complex code. When you keep things simple it will usually save you time when writing it, but not always. What is almost guaranteed, is that it will save everyone who has to read your code (including the future you!) a lot of time. So even if it would take you longer, it’s an investment well worth the effort.