#9 — The Horror, The Horror

This post will be as boring as dirt and likely hard to understand unless you are a code geek. You have been warned.

I deal with a great deal of legacy code in my job as a systems developer: we have dozens of arcane PHP scripts which were never documented, and even their authors couldn't tell you how they work. Granted, that might be related to the fact that some of our alumn employees did a great deal of their work drunk. Our biggest app, which is used by the HelpDesk and all of the labs on campus to manage employees, is perhaps the most (or least, depending on how you look at it) auspicious example of these things in practice.

One of the most heavily used and poorly written pieces of this application is the scheduling functionality: managers create empty schedules with shift slots, and then people schedule themselves to work in empty slots. The resulting schedules are drawn in a big pretty grid with colors and everything so that people can see at a glance who's supposed to be where when. Unfortunately, the script responsible for actually drawing the grid is nigh incomprehensible, and huge at 1111 lines. In order to determine where it is supposed to actually draw a header cell in the grid, it doesn't use an HTML <table>, which would be simplest, nor does it use something simple like saying "well, the last cell was over here, so if this one is supposed be right next to that one I should put it at here+something." Oh no! Either of these solutions would be far too sensible and simple for this little script. What it does do is this:

$day_name_position = ($num_slots_cumul*SLOT_WIDTH)+($realDayOfWeek)*TIME_WIDTH + ($realweek*7)*TIME_WIDTH + $day_name_offset;
if($day_name_position < 0)
{
  $day_name_offset = -$day_name_position;
  $day_name_position += $day_name_offset;
}

Let's ignore the complete lack of consistency in variable name style. Let's also ignore for a moment that the if statement there will always, when $day_name_position (which specifies where the left edge of a header cell should go) is negative, result in $day_name_position being zero. Now Let's ignore that it manages to accomplish setting $day_name_position to zero in two lines when one would have suited just fine. Instead, let's look at the first line. What. The. Hell? What is this code even doing? Even if I told you what the values of all those other variables are, and how they're calculated, do you think it would make any sense? Hint: it wouldn't. But I'll tell you anyways, just for fun. $num_slots_cumul is the number of schedule "slots" drawn so far. So that, times the width of any single slot, seems fair enough. That should tell you about where the next one should be drawn, don't you think? Apparently not. $realDayOfWeek and $realWeek are, you guessed it, integer representations of the day of the week and the week of the year for which we are drawing a header cell. Why are these necessary? Why on earth should which week of the year we're drawing a schedule for affect how we draw said schedule?

Asking myself all those questions gave me a headache. So I decided to not even attempt to answer them, and instead nullify them by simply removing all the code that was there and replacing with code that worked the way I thought it should. I figured that if there was some obscure reason that the week of the year actually mattered, my code wouldn't work and I would know. So I replaced the above (and some other stuff) with the following:

 $last_box_properties = array("width" => 0, "left" => 0);
...boring stuff...
$day_name_position = $last_box_properties["width"] + $last_box_properties["left"];
...boring stuff...
$last_box_properties["width"] = $day_name_width;
$last_box_properties["left"] = $day_name_position;

Isn't that a bit nicer? Turns out it works perfectly too, at least so far. Of course, I still have the other 1105 lines of the script to try and understand and fix.