This final post about code-review, will be dedicated to the various excuses and their refutation.
It takes too much time
The first excuse is the one I most strongly refuse to accept. Saying that code-review takes too much time implicitly implies that it's a waste of time. In my opinion, a software-engineer's work is to provide quality code in order to allow future maintenance of the software. A lot of other things take time - like talking about and writing designs, like monitoring our app and let us not forget automatic tests that require us a lot of time as well.
Code review is not a waste of time - it's an investment of time that will later save us much more time.
I'm not familiar with the code
I relate to this excuse more as more of a "reason" or "concern". That's true, it's harder to review code that you're not familiar with. But it's much easier than you'd might expect. It's so easy to spot code-smells - too many branching in a method, long methods/classes that do much more than they should, bad-namings, irrelevant and misleading comments, etc...
Let's look at the following code:
Almost every java developer would see that concatenating over and over will cause the heap to be filled with unused objects and the GC to work harder (let's assume no compiler-optimization). These examples (maybe a bit less trivial) happen a lot, and they need you to spot them.
Another great insight you can provide would be in a form of a question. "Did you run tests?", "How does this change affect other components?" and so on. These insights are very valuable. When we code, we don't always remember the full-picture around our feature, and it's great to have someone reminding us (before we discover the issues the hard way).
It's too subjective
That's true. Code-reviews are subjective. And yet, researches show an average of 82% of agreement amongst reviewers. This means that in 82% of the times, it's not that subjective. Also, you could always add more reviewers to help you decide when there is a difference of opinions. And lastly, you don't have to agree, but at least hear what others have to say. It might be useful.
It's just a minor change
Oh, what a devious excuse is this one. No matter if we're totally noobs in software-engineering, or we're experts with 20 years of experience, we fall for this one time after time. We just wanted to change a little string, maybe even a log line and BOOM (!!!), an exception is thrown. Let's take this "minor" example:
Won't it be sad when the NullPointerException will be thrown?
In our line of work, even this minor change can cause problems. Problems that could easily be avoided during a code-review.
If you've come this far, I hope I managed to convince you that code-review is something you should embrace in your organization. The best way to start is just doing it. Stop making excuses as to why it won't work and just start. The rest will follow.
Hope you enjoyed and learned from this 3-posts-long topic. In my next post I'll talk about refactoring of legacy code. Stay tuned ;)
Find me on Twitter: @AviEtzioni
More interesting posts from this blog:
No comments:
Post a Comment