Don't let programmers alone
Christian Baer
Posted on October 4, 2017
Recently I discovered a small piece of code that looked like this:
function isElementSpecial(htmlElementToBeChecked) {
var result
if(document.querySelector(htmlElementToBeChecked).classList.contains('special')) {
result = true
} else {
result = false
}
return result
}
I cringed.
My first thought after my initial shock was: 'What on earth was this developer thinking while writing this?' I had to look at this a couple of times to make sure that the above code really only does one thing. Checking that an HTML element has the class special
.
Since I had no time to investigate the how and why this code came to life, I did what every programmer would have done and quickly condensed the code to its essential core.
function isElementSpecial(htmlElementToBeChecked) {
return document.querySelector(htmlElementToBeChecked).classList.contains('special')
}
Since I know whom amongst my colleagues did write this code, I was tempted to ask him straight away why he was writing a simple check in such a complicated manner. Luckily he had already gone home. So I had some minutes to think about what went wrong.
Some facts
- The code is from our front-end test system
- The responsible colleague is a senior programmer with some experience in that field
- He built our testing framework on top of CasperJS by himself
- He has his hands full with keeping our front-end tests stable
Look at point 3. He built this by himself!
There was no one to review, question or criticize his code.
In my opinion, this could have been prevented by someone reviewing his changes in regular and short intervals.
We as programmers build complex (and often large) systems. Having a complete in-depth knowledge of all lines of code is impossible, even if one single person wrote them all. Consequently, it is only natural to forget about that one line or method you wanted to refactor or rewrite. Also, there is a higher risk of getting attached to your code. Why this is a bad idea is excellently explained here.
There are only two methods known to me to reduce all of this. The first one is pair programming and the second one is code reviews. If for whatever reason pair programming is not an option (e.g. distributed team), that leaves only code review.
There are many good 'manuals' for the dos and don'ts of good code reviews (e.g. This).
At my workplace, we do team code review, although most of our code is done while pairing. Every morning we ask ourselves 'do we have something to show the team' and then the projector is turned on and we have a good half an hour of looking at code diffs and asking questions. This often enough reveals useless comments, ambiguous variable/method names and other code styling issues. We also have a general idea of the code that our team produces and our bus factor is increased drastically.
In short: Do code reviews and do them often. There is a high chance of making your code understandable, increasing the bus factor and reducing the attachment to the code.
Posted on October 4, 2017
Join Our Newsletter. No Spam, Only the good stuff.
Sign up to receive the latest update from our blog.