{"id":651,"date":"2014-11-19T00:41:04","date_gmt":"2014-11-19T05:41:04","guid":{"rendered":"http:\/\/www.rfdm.com\/blog\/?p=651"},"modified":"2014-11-19T02:23:09","modified_gmt":"2014-11-19T07:23:09","slug":"dont-do-this-to-your-software-team-example-1","status":"publish","type":"post","link":"https:\/\/www.rfdm.com\/blog\/?p=651","title":{"rendered":"Don’t do this to your software team, example 1"},"content":{"rendered":"

It’s been ages since I’ve posted to my blog. But yesterday I found something in some code at work that prompted me to start a new set of posts.<\/p>\n

The following is an example of something gone awry and then never reviewed or repaired until someone “discovered” it (in this case, me). Since the original author of this bit of code no longer works for the company, I am not aware of the motivation for this original form. My best guess is that it was boilerplate code pasted into their editor of choice, then later committed without review or correction. Here’s the original code:<\/p>\n

\"BadCode\"<\/a><\/p>\n

There are a few troubling things about this code. The first is that the logical equivalent, which I used to replace this code, is an easy to understand one-line function:<\/p>\n

\"BadCodeReplacement\"<\/a><\/p>\n

The second troubling thing is that only a few seconds were required to recognize the bad code and its replacement, and another few to type in the replacement. Even without my daily allowance of coffee, this falls into my 15-second refactoring category.<\/p>\n

The third troubling thing is the suppression of QAC message 6130. QAC is a static analysis tool, one of several we use to find potential problems in our software. Message 6130 is for cyclomatic complexity. I’m not going to explain that here, I’m only going to point out that the general response to a warning from a static analysis tool should not be “Just suppress the warning.”<\/p>\n

The last troubling thing is the clearly incorrect comment associated with the suppression of the static analyzer warning.<\/p>\n

It’s easy to argue that any decent optimizing compiler would recognize the intent of the first version as being the same as the replacement, and produce equivalent assembly. And in fact the compiler we use did just that. That’s not the point. The points are really simple:<\/p>\n