Rescue Job (Code Review) - Process Description

[swatts]

Hello My Lovelies,

2 or 3 times a year we get asked to rescue a project that's gone awry and I thought it might be interesting to discuss our process and highlight some common situations that arise.

I usually starts with a phone call in which the project manager will say....."it's nearly finished, can you come and review the code as the developer(s) have left and we have a deadline in 2 weeks"

And this is a red flag.

Good developers don't leave just as the project gets interesting!

Next we need to know what we are reviewing the code against.

For me, being a practical sort, I want to see the entire system working first. 

Here's a list of questions you can use to establish what you are reviewing the code for. 

Question	Weighting (1 least, 5 Most)
How operational is the system?
How much software maintenance will need doing on a regular basis?
How many software changes will be expected on delivery?
How many software changes will be expected during its lifetime?
Is being added to important?
How risky is the project? (legal or safety)
How close to or past the deadline are we? (Timescale)
How important is ongoing system support?
Cost (how much of the budget is still available?)
How much spare capacity in the system? (Performance)
How important is robustness?
How close to meeting customer requirements is it?
How many deployed systems will there be?
How difficult will support be?

Next we need to look at project management, generally there are 2 scenarios here (and many places in-between)

Incumbent Team

Here you have the additional difficulty of criticising the work and management of people to their faces. This is never easy and rarely successful, because there is an investment in the status quo. If the status quo was working we generally are not called in.

New Team

There will usually be very little domain knowledge, but they will be more willing to make the hard choices.

A final consideration is that as soon as you (or your company) does any work on the code-base you will own and be responsible for 100% of the code-base. With this in mind you have to think about the impact this will have on your business, because maintaining someone else's code is unpredictable, expensive and usually very frustrating.

Brutal Honesty

It's easy to blame the unscrupulous developer for lying on their CV, or just being a bit useless, but in our experience the main culpability lies with poor project management and lack of oversight. Generally we sit the customer down and explain to them these failings and describe how we don't want to work in that fashion.

We haven't even looked at the code yet?

If the customer is still serious about sorting the project out, we're ready to go...

The next stage is to apply some simple engineering process....

Project Clean-up

Before we look at the design we need to ensure the project is in a state to be engineered.

This means that we need to know what version of the source code we're looking at. 

LabVIEW Life Lessons #6 - Versions

Next we need it portable (or under Source Code Control it depends on the work involved)

LabVIEW Life Lessons #1 - Project Portability

Now put in Source Code Control

The we tidy the project

A Tidy Project is a Happy Project

Finally we can look at the design.... Generally our bias is either a light-touch (if the code is redeemable) or a rip out the technical bits approach. The amount of work we do depends on the results of the survey questions earlier in this article.

With the architecture of the system we follow some design rules or as Fabiola puts it here is where we find the meatballs in the spaghetti.

We always start with Coupling, Cohesion and Information Hiding. Coupling is usually the main cause of project designs going badly wrong. The common factor here is overuse of Globals, Locals (on massive block diagrams), FGVs, Classes used to hold data and references, of clever tricks to hold global data and references (DVRs, Files, dBs, Queues, Shared Variables).

Next we look at external dependencies - to libraries, files, MAX etc etc

Cohesion and Info Hiding is mainly bagging up of like functionality (and shared data), encapsulation is what the cool kids are calling it.

Information on this is scattered all over my blog, go look it up.

All of this heart-ache can be solved at a fraction of the cost by following these simple practices...

1.  Employ good engineers (not good software engineers...GOOD ENGINEERS!)
2. Ask them regularly how they are doing and get them to show practical demonstrations that they are progressing as promised.
3. Sack them if they lie or are evasive.

Oh and don't expect us to sort out your crappy projects on a fixed price.

Lots of Love

Steve