Software, technology, sysadmin war stories, and more. Feed
Friday, July 22, 2011

Code written by the lazy and/or crazy

I've seen plenty of code that would have qualified for a Daily WTF entry but couldn't be shared due to ownership issues. Some projects seem to attract more of this than others. I haven't yet determined if this is a function of the programmers involved, management, or just something evil about certain problem spaces that turn people into nutcases with keyboards. It's probably some combination of all three.

My latest example comes from a project which was a pretty big chunk of code. We're talking 150,000 lines of C++ alone, plus a lot of other scripting type hacks wrapped around it and under it in various supporting roles. Couple that with an apparent disdain for using library functions and a "not invented here" streak that must have run very deep and you might start to get the picture here.

One thing I had noticed was that not all classes had a nice division between the .h and the .cc files. I was never a huge fan of C++, but when I had to use it, I didn't mind that I had two files to manage and all of this. Whoever wrote this code, however, decided that splitting things up was not his cup of tea. He'd put the whole thing in the .cc file. That means both the declarations (which would normally go in a .h file) and the actual code were crammed into the same file. Sometimes there would be multiple classes crammed into a single .cc file like this.

As someone who liked to keep one class to a pair of files, this was pretty strange. I figured, okay, this seems to be used when this guy wants to have a class which only other things in this same file ever use. It's strange, but... whatever. I don't want to deal with it right now, and there are too many of them anyway.

Then, one day, I couldn't keep ignoring it. I was trying to untangle a big ball of dependencies, and something broke. Suddenly, some test would no longer build. It couldn't find some class definition. That was strange, so I started digging. My internal monologue went from surprised to annoyed rather quickly.

Hey, this thing is using a class and it's not including class.h! But wait, that class is one of the weirdos which isn't in a .h file. It's only in a .cc file. So how is he getting it in this test? Is he... doing some linker craziness? Or... uh... oh no.

I went and looked a little more closely at those #includes.

#include "pr/ar/ke/someclass.cc"

Yep, this nut case actually was including an entire .cc file. Rather than splitting that class out into its own pair of .cc and .h files and all of that, he just sucked in an entire blob of code into an unrelated module just so he'd get the class definition.

It was around this point that I realized how dangerous a closed loop of code reviewers could be. That, however, is a post for another time.