September 7, 2011
The Right Way To Do Code Reviews Is Not To Do Code ReviewsCode reviews are up there with major surgery and moving house among life's stressful experiences.
The typical code review, based on my own experience and war stories from friends, seems to be that the team gets together in a windowless, airless room with a copy of the code, and expresses opinions about what other people have written. And, as we all know, anything that involves programmers expressing opinions is unlikely to end well.
Adding insult to injury, code reviews tend not to lead to actual improvements in code or design quality. It's rather like being judged on X Factor: the judges have their say, but since music is actually not discussed in any meaningful or constructive way, you leave non the wiser as to what to change.
It doesn't have to be this way, of course. here are my tips for more constructive and productive code reviews:
1. DON'T HOLD CODE REVIEWS!
What? Well, the fact that you're having meetings about code quality is an indication that you're already doing code quality wrong. Do you have Unit Test Execution Meetings, too, where the team gets together and decides which tests are failing? Code review is a continual, ongoing activity. Not a meeting.
2. Set clear quality goals for code and design
It all gets a little less arbitrary and opinion-based when the team have agreed what it is they're actually aiming for. Are you aiming for simplicity? Then when reviewing code, you should be looking for lack of simplicity. Aiming for readability? Seek out code that is hard to understand. Aiming for scalability? Look for processes that can't be easily run in parallel, or load-balanced or clustered. Need a small memory footprint? Need fast response times? Etc etc.
3. Write tests for your quality goals
Need a small memory footprint? How small? Write a test that will fail if the footprint exceeds tolerances. Need code to be as simple as possible? Write a test that will fail when a method or class or package or system (or database schema, or config file) gets too big or too fiddly.
4. Run the tests for your quality goals at last every time you check your code in. The build is broken if any of them fail.
5. Review your code quality tests regularly. If the goals change, change the tests. But remember, raising the quality bar as the code evolves is much, much harder than lowering it. Which is why you'd best start with bar set as high as you can realistically manage, and try and keep it there.
6. Program in pairs. If code's not worth continuous review, it's not worth writing and maintaining.
Posted 7 years, 2 months ago on September 7, 2011