code review

Thoughts on code review

Code review, Test-Driven Development as well as Continuous Integration are considered must-have requirements in terms of ensuring code quality these days. Most companies already have process in place just to make sure that developers are following these practices on daily basis. When it comes to TDD or CI rules are simple, you just need some practice and common understanding and you are ready to go. Correlation between time spent on improvements and effects is clearly visible in the short and long run. So even if your process is not perfect most probably you will benefit from it to some extent. Doing peer review is much harder simply because…

Code review takes a lot of time!

And since this is the most scarce resource that we have nowadays, especially in IT world where every project is behind the schedule before it even begins, we tend to cut corners. So instead of doing great peer reviews, programmers quickly go through code just to verify that code conventions are met, tests are passing and no obvious bugs are present without focusing too much on design itself. As effect companies do not achieve amazing ‘X reasons to do peer review’ that can be googled easily. At least that is what I saw for the past few years…

I’m not going to tell you how to do amazing, mind-blowing code review because there is more than one way of doing so and you have to find your own. Instead, I’m going to give you a small tip that will change the way you approach code reviews in the future simply because

Code review is like hacking of someone else’s idea

This is how we think of it in Pragmatic Coders and here is why… Mistakes that are easy to spot, are equally easy to repair. One man in one day can do an enormous amount of renames, method extractions, style improvements and bug fixes provided that he has the right tools and attitude. What one man can’t do is to re-arrange architecture of your system, fix performance issues that were introduced by some reckless design, reduce inconsistency in data on live production system just because it can’t be stopped for few hours in order to rename the table or column. Naturally I”m not saying that you should cross out simple checks from your reviewer’s list but a small shift in thinking can really make a great difference. Understanding why the change was made in the first place, what possibly can go wrong, what is the strongest and weakest point of a new design, how you can extend it in the future are the keys to successful maintenance and development of a relatively complex system.

The best reviewers that I have met in my life were always focused on long term benefits and overall design consistency more than on tiny, clean code “quirks” and this is exactly an example that we all should follow to make our products even better.



COMMENTS3

  1. Quite nice description of what should any code review focus on. I would just add to this that such reviews should (can) only be done face to face. When making a review we are dealing not with a code, but rather with an idea that was developed by the person that was writing this code. This requires empathy and can not be robbed of nonverbal communication

  2. Totally agree, well put.

    Even if it’s not possible to meet with someone face to face we should use every available means of communication aka Hangout, Skype or Phone to minimize distance.

  3. > more than on tiny, clean code “quirks”

    Those (and not only those) can usually be caught with Automated Code Review; there are plenty of tools that can be configured as commit hooks to catch them (Codacy being one of them).


LEAVE A COMMENT