Peer Review – I write unacceptable code

I have so much stored up that I need to write about that I shall be triggered by random occurrences.

On Thursday Joe told me (and colleagues) that some of the code I had written in our Microsoft Chem4Word project was rubbish.

He was right. It was. I told him so and thanked him and the rest of the project.

How could he do this? I’ve been writing code for decades. The answer is simple – everyone writes unacceptable code. Not always, and not always at the end of the process. The reasons are many.

We are a smallish group, working under time pressure (what’s new about that) and with a new technology. Our collaborators are half a world away in Seattle. Nine months ago we knew nothing about C#, WPF, .NET, XAML, and had only just hacked our way through bits of DOCX. They knew nothing about chemistry, chemical editing idioms, validity, etc. We are both going through a rapid learning process. It’s inevitable that our code is unacceptable. Among several other techniques we need Code Review. By chance I came across:

The Case for Peer Review
published by SmartBear software. Just an intro, but it makes the case that Code Review saves time and money. Lots of it.

So what is it? WP defines it as:

Code review is systematic examination (often as peer review) of computer source code intended to find and fix mistakes overlooked in the initial development phase, improving both the overall quality of software and the developers’ skills.

and gives examples as:

Lightweight code review typically requires less overhead than formal code inspections, though it can be equally effective when done properly.[citation needed] Lightweight reviews are often conducted as part of the normal development process:

• Over-the-shoulder – One developer looks over the author’s shoulder as the latter walks through the code.
• Email pass-around – Source code management system emails code to reviewers automatically after checkin is made.
• Pair Programming – Two authors develop code together at the same workstation, such as is common in Extreme Programming.
• Tool-assisted code review – Authors and reviewers use specialized tools designed for peer code review.

Some of these may also be labeled a “Walkthrough” (informal) or “Critique” (fast and informal).

It helps that we work in a scientific enviroment. Science tests its assertions against the harsh reality of the world and the equally rigorous process of peer review. We expect to be told that our ideas are flawed and must be reworked. No scientist would stand in front of their colleagues and assert they are right because they are an expert – anyone (even Nobel Laureates) can be humbled by the peer-review process. Many – perhaps most – papers are seriously revised as part of the review process (and whatever you think of Open or Closed access no-one should eliminate peer review). Note, however, that Openness enhances the number of potential informal reviews.

So what do we do when coding? Every day, yes day, we have a 10-60 minute telcon, including Live Meeting and MS’s code development environment (Visual Studio, Team Foundation Server, etc.) We show bleeding edge code. We comment on it. Comments are always seen as constructive. Authors do not try to defend what they have written – rather they ask “what are we going to do about it”. It’s often useful to determine why the code is unacceptable, but not to use that to defend it.

Why was the code I wrote unacceptable? There’d been more than author and my code had been to clean up some of the problems (which it had done). But it doing so I had to check the validity of some of the input. We didn’t have a communal approach for reporting invalidity. I was in a rush so I chose to throw an unchecked exception – at least the code would tell people something was wrong. But it’s very very ucky.

This code will be going out to the world. So it has to be perfect. No, it has to be as good as we can make it with the given resources. But we are communally clear that spending some of the resources on code peer review is one of the most efficient things we can do.