Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is my first post. I have a javascript function that I intend use for validating zip codes and I wanted to know if my way could be made better.

function zipcode (inputtxt)    
{  
    var zipcode = /^\+?([0-9]{2})\)?[-. ]?([0-9]{4})[-. ]?([0-9]{4})$/;  

    if((inputtxt.value.match(zipcode))  
        {  
          return true;  
        }  
        else  
        {  
            alert("message");  
            return false;  
        }  
}  
share|improve this question
add comment

2 Answers

Changes I would make : (but not necessary)
- make sure inputtext has a value, and isn't undefined, so it wouldn't throw an error in your if statement.
- Replace the if statement with return instead. (This is mostly to minimize the code a bit, something your minifier won't do for you).
Except for that it seems ok (not looking into the logic of the regex, since im not sure how a zipcode should be parsed)

function zipcode (inputtxt)    
{  

    if (!inputtxt || !inputtxt.value) return false;
    var zipcode = /^\+?([0-9]{2})\)?[-. ]?([0-9]{4})[-. ]?([0-9]{4})$/;  

    return ((inputtxt.value.match(zipcode));
} 
share|improve this answer
add comment

Um. There is verry little code, so one can not say much about your code. It does, what it should, I assume.

One thing, I see is, that you are alerting a message, which has nothing to do with the validation. So if you are looking for SRP - the separation of concerns, you take the alert out and put it elsewhere.

Of course you could shrink the whole thing down to

function checkZipcode(zip) {
    if(!zip) throw new TypeError("zip is not defined"); 
    return /^\+?([0-9]{2})\)?[-. ]?([0-9]{4})[-. ]?([0-9]{4})$/.test(zip); 
}

done. But whether this is an improvement or not is open.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.