Embedded Files
Post date: Dec 19, 2011 3:51:28 PM
This is the worst code I've ever encountered. It's fantastic - I can only fault it as an exercise in confusion in that it doesn't name it's variables after operators - but they are named in a way that's misleading, nonsensical, and prone to typo-error - so it's still awesome.
I should say that I've reformatted the code, because otherwise it makes my eyes bleed, and I'm lawsuit-adverse - just imagine the code with funky wrapping, inconsistent indenting and sneaky brace placement that infers the wrong code blocks, and you're at the right place in your mind.
Off the top of my head, the tactical comment a) describes another function (or missed the '2'), b) makes no sense - in English at least, c) describes a looping/tree-walk process, when the code is entirely linear, d) makes no sense even applying the 'telephone test', e) uses terms 'step', 'parent', 'level' etc. that are either the same, or different, to each other, or common algorithmic terms, f) the term 'withoutany problem' means something, to someone, once.
I just love the effort put in to do a tactical comment (though the coding standard mandated it!) but then the lack of grammatical sense, programmatic sense, or good spelling just undermines the whole thing. I can read code, but theis author sets a trap for the naive by in providing documentation that entirely throws you off course and messes with your initial perception of what going to happen in the code.
I really like that the last line (the 'punch-line') is entirely readible and makes complete sense, in isolation, leaving the reader dazed when they try to link it to the previous statements. Brilliance.
The very best thing about this code is that it came from the place with incredibly high standards of software engineering - in every respect, the place really did do things 'right', without exception. However this code was inherited, and came from another company division - and here, obviously, be dragons....
schedule_com2(step)
int *step;
/* *******************************************************************
* Schedule_com schedules follows following algorithm to schedule *
* jobs : *
* 1) At a particular level 1, say there are m steps ( m >= 1) *
* -scheudle all he jobs in steps m-1 to the machines withoutany*
* problem. *
* -for the last step, schedule job if its parent has been done.*
* 2) Once all the jobs have been done, move over to the next level*
******************************************************************* */
{
if (*step != *stepsAtLevel) { /* Just submit as many jobs as
* possible */
(void) scheduleJobInMask(step);
}
else if (*step == *stepsAtLevel) {
(void) scheduleJobInMask(step);
}
}
A collegue of mine (Keith Robichaud, a great S/W Eng, and not the original author of this code!) first brought the code of build_set/build_server.c to my attention around June 29th 1994. Keith noticed that the second 'if' test was redundant, and asked me to rubber-stamp review his check-in back to the source-code control system - however I spotted that this function, a testament to amazingly bad coding, was entirely redundant, and was in fact only called from one place in the entire codebase. Consequently, the only call to schedule_com2()' was replaced with 'scheduleJobInMask()' and all the this code above was deleted.
We never confirmed the original author, and SCCS records were pruned at a major permanent fork some years before, but I have my suspicions.
....so though I went on to become a good and wise Software Engineer, this code lives now, only in my mind......[and here!]
Page updated
Report abuse