Warning, Geek Rant Approaching

Creating code and sharing it with the world is ALMOST always a good thing. I'm not trying to make it sound as though I have never written bad code, in fact far from it, there's very few pieces of code in my past that I'm proud to say I've written. And sure, there was a time when I probably thought my lousy code was really really good. So don't think that this is a personal attack on the author of the following code, although I am presenting his work as a cautionary example of what NOT to unleash upon the world.

A little back story. I've been working on a simple website for a friend of mine for, oh, say, 6 months or so. He's a photographer so the emphasis of the site is on the photo galleries, and I've gone through countless iterations (self-produced galleries, pre-built galleries in javascript and flash), and really haven't been totally happy with any of them. Then I found a gallery that looked just beautiful. It was written using the jQuery javascript framework and the creator made it look very slick using a reflection library that made the displayed image appear to be floating above a lake or a mirror. Classy.

So I downloaded this gallery-that-shall-not be named, followed the minimal instructions that were provided on the home page as far as setting it up... and of course it didn't work. I spent a little time fiddling with the options set on the page itself and when nothing worked (no image displayed, no thumbnails displayed, no list of galleries displayed) I decided to dig into the code and figure it out.

Step 1: Get the list of 'galleries', or groups of images. I figured if I could get this to display, maybe I could get to some thumbnails and who knows, even get an image to display?

init_menu = function(g) {
var currNav = null, currSubnav = null;

$('div#mainNav li.nav').each(function() {
var nav = this;

nav.subnav = $(nav).find('ul');

$(nav).click(function() {
// hide previous subnav, if any
if(currNav && currNav != nav) {
$(currNav.subnav).BlindUp(300);
}

// show subnav, if any
if($(nav.subnav).css('display') != 'block') {
$(nav.subnav).BlindDown(300);
}

$(nav.subnav).find('li a').click(function() {
if(this != currSubnav) {
try {
var imgs = images[nav.title][this.title];

if(imgs.length > 0) {
g.init_gallery(imgs);
}

if(currSubnav) {
$(currSubnav).removeClass('active');
}
$(this).addClass('active');

// get rid of annoying dotted border on focus
// for hyperlinks
this.onfocus = function() {
this.hideFocus = true;
}

currSubnav = this;

return false;
}
catch(e) {
alert(e.message);
//alert('SYSTEM ERROR: The selected category (' + nav.title + ' | ' + this.title + ') contains to images!');
}
}
});

currNav = nav;

return false;
});
});
}


What you see above comprises the entire contents of the menu.js file. This file's job is apparently to create a series of menus / submenus and allow you to select and initialize a photo gallery. However, the example HTML document that came with this project didn't actually achieve this , in fact it didn't do much of anything. Clearly in a situation like this some comments would be nice. As you'll notice, there are exactly 3 comment lines (lines starting with '//'). The first two are:
// hide previous subnav, if any
// show subnav, if any

The last one is apparently just to hide some unwanted code. The previous two however, are almost as useless in informing an unacquainted user with how to use this code. First question: 'What does subnav mean?' A: A sub-menu, probably. The programmer uses the variable names he came up with in the comments, rather than what data / objects those variables represent. And it only gets stickier because, as you see in this block of code:
var currNav = null, currSubnav = null;

$('div#mainNav li.nav').each(function() {
var nav = this;

nav.subnav = $(nav).find('ul');

'Nav' is apparently this programmer's favorite word. currNav, currSubnav, mainNav, nav, nav.subnav... yikes! Either this file should have been renamed 'nav.js', or every instance of 'nav' should have been replaced with the word 'menu', I think.

Trying to parse the meaning of all these variables (and page elements as well) with very similar names is tricky enough, but trying to do so with only two programmer comments (both of which make reference to 'subnavs') is downright obnoxious.

Here's the lesson. As someone much smarter than me once said, "Real programmers don't write documentation, the code should be obvious". I concur with this statement, if you write code and have the forethought to make it legible and understandable, it shouldn't really need any documentation. However, if you write code where the variable names and context are as vague as this, and you intend to distribute it for others to use, do us all a favor and put in some comments.