project management – hackinghat.com

The Heathrow Terminal 5 story is not all bad. Yes it was a bit of a shambles, yes senior management was fired. But today I found some joy in BA’s misery.

A Metaphor

Now don’t get me wrong this isn’t just me deriving pleasure from others misfortune. Although admittedly as a Brit I am innately very good at that. So good in fact that it’s a constant surprise that the Germans managed to invent a word for what is typically a British malaise: schadenfreude.

No, the silver lining of BA’s lead balloon is that T5 has become a common intellectual currency. It’s failure has so clearly underlined the pitfalls of not doing enough testing that I heard T5 being used as an analogy in a recent implementation meeting. A.N.Other said:

“I would not be happy committing to that deadline if we had to cut testing. The last thing I want is for this to become another T5 …”

Nothing gives a better fuzzy feeling than completing a long testing phase. However if testing is getting squeezed out then you have to get management to agree to extending the deadline before you’ve actually reached that deadline. Indeed cutting testing is to invite what Steve Connell has coined as “Wishful Thinking”, and is the 13th classic mistake of software project management:

Wishful thinking isn’t just optimism. It’s closing your eyes and hoping something works when you have no reasonable basis for thinking it will. … It undermines meaningful planning and may be at the root of more software problems than all other causes combined.

Amen.

So, code reviews are great. Get the benefit of some ass-hole telling you that your comments should be C-style (/*) and not C++-style (//) and remind you that the member name ‘mSuckThis’ is not suitable, ever. No really, code reviews are great. It’s just that a lot of times they just don’t work.

The first time I encountered code-review was when my boss of the time had just read some book on how to manage programmers and was keen to inflict it on all his employees. His code-review process was to take all my work print it out and go through it line-by-line. Master and student style.

This type of code-review, in the way that he implemented it, was meaningless. It concentrated on an important but largely automatable aspect of code review and that is: adherence to coding guidelines.

As I see it there are three types of defect that code review is trying to identify:

Adherence to coding guidelines (or lack of it) and inter-package dependencies.
Identification of localised errors: “that loop is infite”, or “that algorithim should be log(N) and not N^2”, “that module is way too big”
Identification of non-local errors. Where local means local to the code-review. For instance the impact of adding a throw on a widely used method and how that affects all the dependent code paths.

I question anyone’s ability to fully understand the dynamic nature of any reasonable sized piece of software by just looking at a small excerpt. Every time you review that code you have to ‘load’ that supporting information into your head to be able to identify whether the code is totally awesome or tragically bogus. In my experience defects of a local type (type 2) were sadly rarely identified by code review and defects of a non-local type (type 3) almost never.

The improvement of code-quality I’m passionate about. But I don’t see any realistic way to achieve what I want. To identify non-local errors you really need your code reviewer to sit next to you during the development or be as deeply involved in the code as you are. It probably would need a similar approach to reliably find local errors too. However your reviewer is rarely as involved as that. It seems that some judicious use of pair programming might be the answer but that comes with its own problems.

It seems that to get the best out of code-reviews you have to be very careful about how you implement them. Sure, let’s automate what we can automate and pair program on tricky items but the real code-review needs to be extremely skilfully handled to get the best bang-for-your-buck-chuck.