1
\$\begingroup\$

I have a function that I intend to 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;  
        }  
}  
\$\endgroup\$
3
\$\begingroup\$

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 (I'm not looking into the logic of the regex, since I'm 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));
} 
\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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