Make Regular Expressions Not More Code
Currently I am bug hunting at work. Product and QA have identify a number of issues engineering needs to fix before we begin A/B testing of our new website design. For the last few months while our website was being made feature complete, I'd been working on both our iPhone and Android apps. They were updated to use some newly available server data and fix some outstanding issues. So I missed out on a lot of the changes made to the site. Now I am catching up on the code by fixing bugs and refactoring code.
While the code in general is not too bad, there were a few things that troubled me. The first of which was a general ignorance of regular expressions. Every developer should learn regular expressions in their languages of choice and learn them well. They save time, minimize space, and increase code clarity. Here is the original code:
zipIsValid: function (zip, notify) {
var isValid = true;
var validCharacters = "0123456789";
if (zip.length != 5) {
isValid = false;
}
if (isValid) {
for (var i = 0; i < zip.length; i++) {
temp = "" + zip.substring(i, i + 1);
if (validCharacters.indexOf(temp) == "-1") {
isValid = false;
}
}
}
if (!isValid && notify) {
window.alert("The ZIP code you entered is invalid. Please enter a valid ZIP code. ");
}
return isValid;
},
While the code in general is not too bad, there were a few things that troubled me. The first of which was a general ignorance of regular expressions. Every developer should learn regular expressions in their languages of choice and learn them well. They save time, minimize space, and increase code clarity. Here is the original code:
zipIsValid: function (zip, notify) {
var isValid = true;
var validCharacters = "0123456789";
if (zip.length != 5) {
isValid = false;
}
if (isValid) {
for (var i = 0; i < zip.length; i++) {
temp = "" + zip.substring(i, i + 1);
if (validCharacters.indexOf(temp) == "-1") {
isValid = false;
}
}
}
if (!isValid && notify) {
window.alert("The ZIP code you entered is invalid. Please enter a valid ZIP code. ");
}
return isValid;
},
All the above only checks to see if a zip code is valid and if not may display an alert box if a flag is set. I replaced it with the following code:
isZipCodeValid: function (zip) {
var re = /^\d{5}$/i;
return zip && zip.match(re) != null;
},
The replacement consists of just two lines: a regular expression pattern and a line to test the expression. IMHO, this code is far more maintainable than the code above, if you understand regular expressions, that is.
Also, I moved the warning display to its own method, since every method should do only one thing. And I renamed the method to make it more readable.
