Knowledge base‎ > ‎

Code review: more politics than technology?

posted Apr 20, 2010, 5:24 AM by Szabolcs Szádeczky-Kardoss   [ updated Apr 22, 2010, 2:10 AM by István Soós ]
A few weeks ago there was a question on a Hungarian Java user list about PMD and code review that made me wonder what is the actual state of automated code review or code analyzer tools like PMD compared to the manual process. The political aspects of the code review process brought up some good and some bad memories too...

On the mail list...
My quick answer on the mail list was around the following: "I wouldn't call PMD as a code review tool, it is rather a code checker. Even if PMD rules pass (especially for the rules actually make sense) we have had projects where our manual code review improved the quality of the code significantly."

Okay, this is not a big surprise so far. But the reply made me think a little bit: "Our actual development methodology doesn't have any place for code review. I am are looking for way to include it without disrupting the current development timelines. For the first 2-3 projects it will be only feedback, to have some routine in it and later on we can fine-tune the process. However somewhere we shall start and PMD looks good for such."

Good luck for such an initiative! :)

On the dark side...
I have some doubts about the long-term success if it will remain feedback only and it is not enforced even in the actual projects. Why I see it that skeptical? I think if something (a) is not mandatory and (b) it is not in the essential culture of the group and (c) it could be skipped to save some time for the hard deadline - will be essentially skipped to save that crucial time... Been there, done that, forgot that... :)

One of our clients did have similar scenario a year ago: a multinational corporation that works mostly with outsourced developers (read: cheaper than everything = not always as qualified as you would like), while we were hired as architects for project assistance. Our first shock was when we've realized that nobody (read: not a single person) has ever required an internal code review of the shipped products. Of course officially they have had it: if the department A of the contractor company developed a product, they have requested a code review from the department B. Guess what: it has passed. Even if they contracted a third party, it was as cheap or much more cheaper than the original one, so there was no surprise they haven't produced any reasonable report. By the way: have you noticed that code review reports make no sense at all for product managers who just know Excel-sheets? :)

Anyway, as architects we were the first to introduce this requirement in the business process, and to be honest, we have made progress in the technical parts, but we have slightly failed the political aspects. At least it reinforced the belief: if it is not mandatory, it will be skipped - period.

In technical terms, we were quite good: we had introduced PMD as an IDE-plugin, so every developer was able to check the rules for itself. Unfortunately PMD contains a lot rules that just make no sense or are irrelevant in the actual project context, so we had to fine-tune the ruleset and eliminate some annoying ones. We have created a few new rules to describe some critical scenario but generally we have left PMD to do the job.

On the political side, we - as architects -  were a bit outsiders: we have had no rights to push the project deadline or block the handover process. Even if the project was buggy (in PMD sense and in our common sense, like String == instead of .equals(...)), we have had no impact on the process. The PMD reports had been sent around, but the management gave it no sh*t (they hadn't understood it, and the deadline was too much worry anyway), so nothing happened with the results. The only thing that successful resonated something was when I've periodically sent a report about the number of errors in the project - with the historical numbers, showing a clearly rising tendency :[. Finally we were allocated with the budget to do some manual code review as well, and it was a clearly better story - well, at least for us, architects :].

And the brighter side...
Speaking on the good experiences, I've had a job at an investment bank, where we had a clever and responsible team of people around infrastructure libraries. These libraries were crucial, many other departments has used them in production environment - so essentially a solid code review process was in place. There was practically no code that wasn't peer reviewed before make to a new release - with a few exception of emergency releases, but they were in a different branch.

Speaking of our internal developments, we have the luxury of doing such code reviews irregularly, because instead of such process, we rely on:
  • unit testing
  • test coverage tools
  • performance profiling that monitors the critical parts
By the way, this resonates to the "automate everything" motto of successful businesses :)

And some supporting tools...
Cutting a long story short (oh, am I late with that?), my view on the technological part:
  • PMD is a nice tool, but has limitations and obsolete rulesets (some of them are detected by the java compiler anyway). If you need similar tools, even open-source, you might check these code analyzers too.
  • Manual code review is always better than such tools. I don't think that static code review for 'feedback' purpose makes any sense without some policies.
  • The weakest chain is - as always - the timeline and the budget. If those doesn't allow the code review process to allocate more time for better quality, you could have the best toolkits in the world, but you might just drop it in the trash.
As I'm typing this entry, I've started looking into tools that allow better formalization and documentation of the manual code review process, basically a better team collaboration. As for one example, Atlassian Crucible looks interesting. Of course there are open source code review tools there, it might be easy to pick from one of those too.

Most of these tools are a mixture of version control repository viewer, issue tracker and commenting, so if you have these installed, you might just use it for the code review process too. Our preferred tool is Redmine, and it is pretty easy to use it for such purpose:
  • When committing code, not only update the originating issue, create a new one requesting a code review and optionally assign to someone.
  • The review process will produce questions and comments, these can be new issues as well.
  • The discussions around these comments can be just as organized or as loosely defined as you like - without much technological restrictions...
And of course: none of these tools can beat the performance of the developers who are sitting at the same desk and checking the code on the monitors real time, while discussing their ideas :).

published: 2009-08-15, a:István, y:2009, l:automation, l:codereview, l:governance, l:pmd, l:redmine, l:testing, l:unittesting