Title: From the Coalface

My friend may have synthesised his story from one or more incidents at different times and places. On the other hand he may not have.

My friend told me of a curious series of incidents that happened over the last week or so on his project. All the source code for a project was being reviewed. Everyone agreed that code inspections or walk-throughs are a 'Good Idea'™. How ever it transpired that this was in the same vein as winning the lottery is a Good Idea. Doing it was another matter.

There were three teams on the project. One doing a core and the other two doing applications that would use the core. As one of the engineers doing the core has had 12 months more experience (on one of the application fields) than the rest he was made the System Design Authority. However his general SW engineering experience was a lot less than the rest of the engineers.

The code reviews were done. One engineer from each application team attended their own code review (and took the notes) With the system design person. A person from the core team (because the app would have to interface to it) and occasionally a contractor who had worked on the target before who was working on the core.

The same scenario was used for the core reviews. This meant that the core was only reviewed by those who had written it! There were no reviewers from the applications. For reasons of time and staff availability some reviews had only two people present.

This produced some interesting results. There were many comments from the application review where the core team had no prior knowledge like "What is ABC1?" ABC1 is a term used through out the SW and documentation including the glossary for the particular application. I.e. it is like a looking at comms SW for a PC and wanting COM1 explained everywhere it appears in the source code… If you do not know what a COM port it perhaps you should read the documentation before adjusting the code?

However on the application where the core team did have prior knowledge it was not felt necessary to explain any acronym no matter where it appeared because "everyone knew what it meant!" One engineer did raise this when he saw some unexplained names in the code of a fellow team member but the Team Leader said "I know what that is... no need for a comment."

Now this brought about some interesting actions. The Core review went well with very few comments. One application had a few more but, strangely, in the application where the core team had no understanding there were a lot of comments.

Enter the Project Manager. My friend as the lead on the application with the most comments was called in to explain the problem.

My friend called me that evening and mentioned the above so I called someone I know who had some dealings with the new MISRA Embedded C Guidelines for automotive SW. Apparently one of the mandatory rules does require initialisation of automatic variables before use and suggests doing it at declaration.

NOTE: MISRA is the SW arm of the MIRA who do all the independent car testing. If you are doing any embedded work I can recommend getting hold of the standard. At £25 you have no excuse. It actually makes good sense for any C programming not just embedded or safety critical. You can get the standard from David Ward: <david.ward@mira.co.uk> or 01203 355430

My friend renewed with the knowledge that he was not loosing his sanity went back to the project leader to say "Not only is this not bad practice it is mandated good practice. There is an embedded C standard that says so!" The reply was "I'm not interested in standards I have a project to get out..." My friend persevered and again argued that in fact the SW was not of poor quality but to no avail. It must be poor quality as it got more review comments than the core. So my friend took the same function from the core code as had been looked at previously and found several dubious bits of C (without even resorting to lint) however he was told that this was just being picky as the SW worked...

Foot note: When the core code was first compiled for a build it took 2 days to get it to compile and link. Both the applications (using their own stubs for the core) had been able to compile and link at all times during development despite being of "poor quality".

What is really disturbing about this tale is that the company is a household name and one where you would expect better than average management. There is also the two serious breeches of code inspection rules: code inspections are for code (and coding) improvement and not to assign blame, code reviews must not be done by the owners of the code (though they should have a representative present).

I have a simple question, but one that I suspect cannot be answered. How should we handle such situations? Note that the core code group were far from innocent.