Wednesday, July 18, 2012

Comments

Consider the following code:

//Increment data count here
dataCount++;

The developer responsible for this legacy code heard that comments are good. What he was not aware of is DRY (Don't Repeat Yourself) principle. It is obvious what the code does. What is not obvious is why it is incremented here and not at the end of the function. I'd like to know why it is incremented unconditionally. As maintenance developer I'd like to see variable name that clarifies what kind of data it counts. Unfortunately there is no comment to help me.

Sunday, July 15, 2012

Phunction names.

Somewhere in code there was a function named "abtain_list_lock", That's right, with "a". A few years ago a rather large group of developers branched out from main version to work on a another set of special awesome features. The developers working on side branch quickly renamed the function to "obtain_list_lock" and continued working. Over the next three years much work done; lots of new functions that obtain list locks were written. It was time for a merge between branches and resulting version included awesome new features from both branches. It also included both "abtain_list_lock" and "obtain_list_lock" having the same function body. One day this will be fixed, but not today.

Why?
1. Developers do not read function and variable names carefully, IDE allows code navigation without reading function names.
2. Developers on side branch expected that the typo on main branch will be fixed by the next person who uses that function, extrapolating own quality standards on others. It also highlights some bureaucracy in branch ownership and commit policies in larger software corporations.
3. C language allows implicit function declarations. Many users of "abtain" did not include the relevant header file where function is declared. Thus removal of function with typo led to errors while linking, not during compilation. Due to complicated custom build system such errors did not show verbose error messages by default. A simple preprocessor macro in header file wouldn't be enough to make it link.
4. The merge was done in haste. People were under pressure to release product that contains all features from both branches as soon as possible.