I received a package today that contained 6 coloured rubber-bands courtesy of the guys at http://clean-code-developer.de. It felt bad to shake out the contents of that bag and still be putting on the Red band. I thought I'd be well into orange by now but life keeps getting in the way. Ah well, like trying to learn my guitar, practice makes perfect I guess.
Today's rule is to practice Root Cause Analysis. This is the practice of spending time to discover the underlying problem instead of applying a quick-fix band-aid solution.
Too often when novice developers are pointed at a bug database they will crack open the application into the debugger, cause the issue to occur and then apply a fix right where they are in the debugger. Sounds great right? After all the problem goes away, the client is happy and all is well with the world. Or is it?
Have you ever opened a class that had a method in it like this:
public void SaveChanges()
// Client says that sometimes Order Lines aren't saved
foreach (var line in Order.Lines)
The problem with code like this is that it fixes the issue that the client was having but not the defect that was present in the code. In effect the changes prevent the symptoms from appearing but do not address the underlying problem. If the specification for the Repos.Save method states that it will save OrderLines when it saves the Orders class (as is implied by the sample above) then other developers are likely to encounter a similar issue in the future. When they do they are going to track down the author of the above snippet and ask them how they solved it. Nine times out of ten they will say "Oh yeah, I had that issue with the Checkout screen but I can't remember what I did. Just crack it open and copy whatever I did."
The correct way to approach fixing the above defect would have been to fix the Repos.Save method. In fact it turns out that in this instance the defect was caused by Repos.Save method checking the IsNew property instead of the IsDirty one. This meant that brand new order lines were persisted but not changes to existing ones. A simple mistake with a simple fix.
Root Cause Analysis is a technique for peeling back layers of a problem until you discover what is actually at the heart of it (the so-called Root Cause). The idea is that it is always better to address the Root Cause as early as possible otherwise it will continue to manifest throughout various areas of your applications and secondary and tertiary symptoms.
One of my favourite techniques for performing Root Cause Analysis is the 5 whys method. It's a simple enough idea, state the current problem and then ask why. Then again, and again, and again and again (5 times). In fact, with the above problem this turns out some interesting results:
- Order lines are not always persisted on the Checkout screen
- Why? The SaveChanges Method does not always persist OrderLines when called (The first sample fixed this)
- Why? It defers calls to the Repos.Save method which does not always persist OrderLines
- Why? Repos.Save only saves lines which have their IsNew property set to true. The original developer should have used IsDirty rather than IsNew. They made a simple mistake. (The explanation above fixed this)
- Why? The names of these two properties are confusing and they are easy to accidentally switch
- Why? The names of these properties do not truly reflect what they represent.
So the problem is worse than we originally anticipated. Although the client came to us with a problem about screen/database behaviour, it seems that our OrderLines class has two properties on it that don't really reflect what they actually do. If we renamed IsNew to DoesNotHaveAPersistedRepresentation and IsDirty to IsDifferentFromPersistedRepresentation then it is much harder to get these mixed up in the future. We could also have added a NeedsToBeSaved boolean property that combined these.
There are some issues with the "5 whys" methods. You have to basically know all the things that could go wrong up front and you can occasionally ask the wrong why question. Nevertheless it is a useful mental tool to bring out when you are faced with a potentially difficult problem as it will at the very worst make you think.
Root Cause Analysis needn't be limited to uncovering defects in code, it can be used to uncover all kinds of issues. For example:
- I am frequently late to work
- Why? I often miss my bus
- Why? I don't have enough time to get ready when I wake up
- Why? I have a hard time getting up when my alarm goes off
- Why? I don't feel as if I have had enough sleep
- Why? I go to bed much too late
So if I get to bed before 2am I may be on time for that 9am meeting? It turns out the answer is yes.
I can't remember where I first learned about Root Cause Analysis and the 5 whys method but recently I've been reading The Art of Agile Development by James Shore & Shane Warden and they have a good section on using Root Cause Analysis to discover and correct issues with the software development process itself. They use the 5 whys technique to determine that the reason they spend so much time getting the code compile when starting a new task is ultimately because their design makes it hard to isolate the application from the database for testing purposes. It's good stuff!
No new comments are allowed on this post.