Jumpy Thumbnails
It was a snowy Saturday morning and I didn’t have a lot of pressing matters, so I decided to track down a silly issue, off the clock, for a client of mine. On two similar pages, the space below a group of thumbnails was different. The thumbnails were <img> tags wrapped in <a>tags, inside a <td>.
I started by using Firebug to see what styles were applied to the various elements on the page. I expected the padding or margin on either the <img> or <a> would be different between the pages.
Firebug showed that there were no differences in the applied styles, even though clearly there were differences on the page. Firebug even “admitted” as much by showing one <a> element grayed out, and showing the other one active. In Firebug, a grayed-out element means that means that it is not displaying. But again, the applied styles were identical, and on both pages the <img> tag were displaying. I noticed that page with the grayed-out <a> tag had less space — in fact, the <a> occupied no space at all, (implying a float? or zero height/width?) where on the other page, the <a> had dimensions.
Obviously it wasn’t a style issue since Firebug reported the CSS being identical. I decided to look at the raw HTML source code (which is generated by PHP) and discovered that one image contained the meaningless “align=’bottom'” and the other contained the equally meaningless “align=’left'” — I say “meaningless” because once you use CSS style sheets, they generally control the alignment, and not these HTML attributes.
Nevertheless, I removed them both. Sure enough, things started lining up.
dry();
There is actually a larger problem at work here. Because why should there be two, different, meaningless ways of doing the same thing?
The code that I found (but didn’t write) repeats two (slightly) different versions of the same 60-100 lines of code. What I found was something like this:
if (logic for top level page)
{
// 60 lines of code to draw thumbnails
} else if (logic for sub-level page) {
// 60 lines of code to draw thumbnails
// 40 lines of code to do some other stuff
}
This is what is known as a Bad Practice. Once you write 60 lines of code, when you need to do the same thing again later, you don’t copy and paste those 60 lines. You encapsulate them and re-use them. If you need to modify 5 lines, you separate the 5 lines that are different and you reuse the 55 that are the same.
Because when you don’t, you end up making a change to one (“align=’bottom'”) that you forget to make to the other (“align=’left'”).
And you end up with a finished product that is buggy and inconsistent and difficult to fix.
Humbleness Disclaimer
This sort of programming mistake really frustrates me. I spend hours debugging some stupid flaw that, really, is hardly worth fixing. I mean, 4 pixels of white-space difference should not be worth an hour of my time. (In fact, I didn’t bill my client for this fix because my professional advice would have been to save their money.)
If the code had made use of a “drawThumbnails()” function, this wouldn’t have been an issue. If the pages are supposed to look the same, they should be made from the same mold. This is why “DRY” (Don’t Repeat Yourself) is an important rule for programmers.
And now that I’ve finished my rant, I’d like to acknowledge that somewhere, out there in the wide Internet, is a post just like this one, written by some poor programmer who inherited my awful code from five years ago.
To you, I apologize. I am always learning and always improving. Unfortunately that means that stuff I wrote years ago is guaranteed to be embarrassing today. Maybe I didn’t get enough time to finish the project well, maybe I was swamped with other work and had to leave well enough alone. Or maybe I was just younger and dumber and there are no excuses. Hopefully you’ll forgive me rather than curse my name.
Let’s all agree to write better code, not release anything that isn’t ready, and not be too hard on the developers who came before us.
