Last Friday (03FEB12) I had the first code review for a major new feature I’m currently working on. The design and implementation of this feature are my responsibility, and I’m the only one working on it, so I feel a significant amount of pride, responsibility, and ownership. From this overly-sensitive perspective, the review didn’t go so well.
The application implements real-time signal processing algorithms running on a cluster of machines configured as a grid. Its completely written in ANSI C, despite the fact that one of the overarching design requirements is that the top-level application have no platform-specific code unless absolutely necessary. I don’t agree with the language choice, but the project has sensible reasons for this (the application has a lot of legacy and only two engineers besides myself (out of a team of 18) know C++, and since no one besides me really knows any other languages). Where I start to butt heads, and get increasingly irritated, is in the design choices.
I write object-oriented C as much as sensible, though I do sometimes break encapsulation to make my new code fit cleanly into the legacy portions. I try to break my code up into small, descriptive functions to make it easy to tell what is going on where. I try to keep variable declarations within the innermost scope they are required in. I make heavy use of
const, and initialize variables at declaration. None of these are appreciated, and some aren’t even tolerated.
Now, I don’t want to give the impression that I whine about having to follow coding standards. I don’t. Standards are necessary, especially on a large project, where stylistic issues come down to personal preference. However, everything I mentioned above has valid, functional purposes. Some are considered best practices, not only in C but most other languages as well. Yet in my leads’ ideal world, none of those elements (except possibly
const usage) would be present in the code.
In this most recent review, I got called on #3 and #4. They want to see all the variables declared at the top of the function, and would prefer them to be initialized at the beginning of the function itself. The reasoning: maintainability. The whole reason for #3 and #4 in the first place is maintainability (and, in a few cases, optimization). However, the code they have been writing and reading for the past 20+ years hasn’t changed measurably from K&R’s style.
The current codebase is unmaintainable, and the corporate coding and design standards were tailored to match the style of all the 15-year-old legacy code the application is built on, to avoid the work of having to update the old code to new standards. A few months ago, I compared the corporate standard to the project standard, and was not happy with the items they tailored. The corporate standard is much more sensible, and we should have updated our codebase as we went to meet it, not make the old code compliant by definition.
I didn’t really want to use this blog for venting about my job, but I’m losing sleep over this tonight and felt writing about it might help.