CodeSOD: Javascript Best Practices

For those looking to get into software development, there's never been more access to tutorials, guides, training programs, and communities of developers ready to foster those trying to get started.

Unfortunately, you've got folks that think because they've written a few websites, they're ready to teach others, and write up their own tutorials without having the programming background or the educational background to effectively communicate best practices to the public.

A few years back, Rob Speed was poking at a course calling itself "JavaScript Best Practices". One slide, quite correctly, warned: "If numerical data comes from a form, it may appear as a string. Parse the data before performing validation." And, by way of example, it supplied this code, for parsing postal codes:

function checkValidZip(entry) { var userZip = parseInt(entry); try { if ( typeof userZip === "number" && !isNaN(userZip) ) { if (userZip.toFixed(0).length === 5) { return true; } else { throw new Error("Nope!") } } else { throw new Error("Nope!"); } } catch (e) { if (e.message === "Nope!") { alert("Please enter a valid zip, dude."); return false; } }}

Now, I'm almost willing to forgive the alert. Sometimes, when demonstrating an idea in code, you just want the student to see something happen, so sure, alert away. I wouldn't call that a "best practice", though.

The flow-control-by-exceptions, on the other hand, is starting to get ugly. It certainly makes the code far less clear. Using the message property of the exception object to filter the exceptions is pointless, since the only thing in the try that could throw an exception is your own throws in the first place.

But there's another problem here, and it's where they check for 5 digits. See, here's the thing, while in the US, we think of zip codes as numeric fields, they're not. And I don't just mean, "Oh, in other places postal codes contain characters." I mean that we don't ever treat zip codes as numbers- you'd never add two zip codes together and expect the result to mean something. Zip codes are just identifiers, and because of that, all parts of the identifier are significant.

Take, for example, Holtsville, NY. Zip 00501 and 00544. If you were to run these two codes through this validation function, it would fail. userZip.toFixed(0) is removing the decimal portion of the number (which, since we parseInted above, we shouldn't have a decimal portion anyway). So the length of these two perfectly valid zip codes would be 3, so this function wouldn't consider them valid.

In the end, this "best practice" example fails on every level: it abuses exception handling for flow control, it provides user feedback in a very not-best-practice way, and it uses numerical validation for data which actually shouldn't be treated like a number.

But hey, I guess they didn't use regular expressions, so they don't have two problems.

What really gets my goat about these sorts of things are that the people publishing trash tutorials are publishing them for people who don't yet know enough to know the tutorial is trash. I'd name and shame, but I couldn't find the specific tutorial that Rob was referencing, so we can hope that it's since been vanished. But I'm sure there's a dozen worse ones, taking it place.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!