I started programming a couple of days before yesterday, so I had several occasions of peeking a look at code written by others. According to Jeff best programmers are those who answer “mine” at the question “Which is the worst code you have ever seen?” Probably I wouldn’t qualify for the elite since I consider my code not so bad. For example, in the past days, I stumbled upon a piece of code quite crappy. Most out of curiosity I analyzed the dependency among modules. Note I am talking about real straightforward dependency, the one defined by module A calling a function in module B. I got the pretty picture you see here:
You may argue that F.c is quite independent, but the truth is that F.c is empty and not used in the project, I just included it in the graph because it was in the directory.
Now it would be just an awesome example of dependency-hell applied to programming (others may call “Big Ball of Mud”) would not be the case I have the task of adding features to this mess.
As every ball of mud of respectable dimensions, a number of anti-patterns have been consistently applied with generosity. For example, around 110 global variables have been defined in a .h file.
Ok, read that sentence again.
Variables have been defined in a header file, not just declared. This brings us to an interesting anti-pattern: each .c file has the corresponding .h file which is intended to be included only once by the .c file. I.e. F.h is the header file for F.c in the sense that just F.c has to include F.h. In this way, each header file has a section for prototypes of the module and a section for externs.
One of the most relevant members of the global community in this code is variable ‘i’. You may argue about the problem of having such a short name in such a large scope. This was my reaction until I realized that such variable was used everywhere for loop indexing. What the f*! I mean … why?! The compiler is expected to optimize away the index of a loop, possibly by moving the variable into a CPU register or by unrolling the loop. This is simply not possible when a global is involved since the compiler is not granted that someone else is using the global or it is affected by some side effect of the loop.
I tried to enter the mind of the original programmer… and I imagined a young at his (or her) first job, arriving a day at the workplace and claiming: “Yo! I had a wonderful idea! Why don’t we use a global for ‘i’ so that we don’t need to declare it everywhere!”
I won’t talk about common anti-patterns such as never-ending function body (900+ lines) or deeply nested (16 levels… a record I guess). The other aspects that stroke me are hilariously long lines and tactical comments everywhere.
80 columns per line may be a bit old-fashioned but I like it and force you to be concise and avoid too nesting, but long lines in this code easily reach 160 columns and I suppose there are some around the 200 columns.
Tactical comments are those notes left by the programmer explaining what every line is supposed to do. I am not fond of this practice that actually reduces the readability but this code reaches really unexplored worlds of tactics. Several functions have each line commented (from column 100 to column 160). If a single comment doesn’t fit in a single line (which could happen when you hit column 200) it is split along multiple lines even if the code no longer relates to the description.
Yes, I am convinced that I am not the best programmer in the world (nor one in the top ten), but I strive as I can not to write code such as this. And the reason is that a) I don’t want to lose my sanity trying to debug it (and possibly change it in the future) and b) I want everyone to live in love and peace… programmers included.