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.

I'm trying to improve my JavaScript (I'm usually a copy/paste guy but can do basic DOM stuff with jquery too), so I decided to try and make a function to validate an email address without using Regex.

The code seems to work, but I'm interested in hearing how it could be improved.

I'll include the code below, but you can run/fork it on Codepen - http://cdpn.io/ixbJD

HTML

<input id="email" type="text">
<input id="submit" type="submit" value="Validate">

JS

// Function: valid email address without regex
function isvalidemail(email) {

    // Get email parts
    var emailParts = email.split('@');

    // There must be exactly 2 parts
    if(emailParts.length !== 2) {
        alert("Wrong number of @ signs");
        return false;   
    }

    // Name the parts
    var emailName = emailParts[0];
    var emailDomain = emailParts[1];

    // === Validate the parts === \\

    // Must be at least one char before @ and 3 chars after
    if(emailName.length < 1 || emailDomain.length < 3) {
        alert("Wrong number of characters before or after @ sign");
        return false;
    }

    // Define valid chars
    var validChars = ['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z','.','0','1','2','3','4','5','6','7','8','9','_','-'];

    // emailName must only include valid chars
    for(var i = 0; i < emailName.length; i += 1) {
        if(validChars.indexOf(emailName.charAt(i)) < 0 ) {
            alert("Invalid character in name section");
            return false;   
        }
    }
    // emailDomain must only include valid chars
    for(var j = 0; j < emailDomain.length; j += 1) {
        if(validChars.indexOf(emailDomain.charAt(j)) < 0) {
            alert("Invalid character in domain section");
            return false;   
        }
    }

    // Domain must include but not start with .
    if(emailDomain.indexOf('.') <= 0) {
        alert("Domain must include but not start with .");
        return false;
    }

    // Domain's last . should be 2 chars or more from the end
    var emailDomainParts = emailDomain.split('.');
    if(emailDomainParts[emailDomainParts.length - 1].length < 2) {
        alert("Domain's last . should be 2 chars or more from the end");
        return false;   
    }

    alert("Email address seems valid");
    return true;
}

document.getElementById('submit').onclick = function() {
    var email = document.getElementById('email').value;
    isvalidemail(email);
};

One thing I know could be improved is where I repeat basically the same code twice to validate the characters in each section. I'll definitely appreciate any and all feedback.

Edit: The feedback so far has focused more on obscure but valid email addresses getting through validation. I appreciate that feedback, but am more interested in hearing if the way I've structured the function and sections could be done better, as the exercise for me was more about writing well-structured JavaScript and getting familiar with some of the built-in functions than about writing a perfect validator (which is also the reason I didn't use Regex).

share|improve this question

migrated from stackoverflow.com Dec 30 '13 at 2:12

This question came from our site for professional and enthusiast programmers.

1  
"I'm interested in hearing how it could be improved" - With a regex. But if the point of the exercise is specifically to practice coding without regex, well, yes, you will end up with some for of loop to test individual characters. –  nnnnnn Dec 30 '13 at 2:45
1  
Your array of validChars hardly covers the spec: spaces, quotes, @, #, >, ... all are valid chars, too. Validation for an email address is something done best server-side, because JS is easily bypassed + Server-side languages are better suited for validation –  Elias Van Ootegem Dec 30 '13 at 12:48
    
HTML5 has some new input types that does not require you to use JavaScript. <input type='email' name='email'> (This is not supported in Safari (yet).) Check the full list of input types. –  ThijmenDF Jun 23 at 13:54
    
@ThijmenDF Please do not link to w3schools. Last time I checked, they were primarily known to spread broken/wrong information. Please link to the nicely formatted and readable standard or for example the Mozilla docs instead. –  Jonas Wielicki Jun 23 at 15:29

3 Answers 3

Read the http://en.wikipedia.org/wiki/Email_address#Syntax for what is allowed..

Your current validation will produce a lot of fails for valid email addresses

share|improve this answer
    
Wow, you really can get away with a lot in an email address... –  GusRuss89 Dec 30 '13 at 2:24
4  
@GusRuss89 - it is generally accepted that there is no better way to validate an e-mail than to actually send an e-mail, and get a response..... At that point, it is valid. This comes up often: stackoverflow.com/questions/201323/… –  rolfl Dec 30 '13 at 2:42

Right off the bat, there's too much explicit nit-picking going on for email validation without using a Regex... but since you're practicing javascript, here's a few tips:

  1. Since this function requires a parameter passed, you always want to make sure one exists before operating on it (i.e. using the split method).
  2. camelCase your method names for improved readability
  3. Declaring your variables on the top improves readability
  4. Optional but suggested: optimize your code for performance (i.e. research best patterns for for-loops, etc.)

You can look up great references from Douglas Crockford on some Do's and Don'ts of Javascript to also improve your skills. Those were helpful to review in the early days. YouTube will have many of his lectures for review.

Also, I agree with Gaby -- emails do accept so many valid characters, including ones we've never thought would be acceptable.

Lastly, here's code that I quickly modified for yours. I'm sure there's a few more things you can do to improve the quality. A regex could obviously replace this entire function with roughly 5 lines of code if you're not picky.

// Function: valid email address without regex
function isValidEmail(email) {
    // If no email or wrong value gets passed in (or nothing is passed in), immediately return false.
    if(typeof email === 'undefined') return null;
    if(typeof email !== 'string' || email.indexOf('@') === -1) return false;

    // Get email parts
    var emailParts = email.split('@'),
        emailName = emailParts[0],
        emailDomain = emailParts[1],
        emailDomainParts = emailDomain.split('.'),
        validChars = 'abcdefghijklmnopqrstuvwxyz.0123456789_-';

    // There must be exactly 2 parts
    if(emailParts.length !== 2) {
        alert("Wrong number of @ signs");
        return false;
    }

    // Must be at least one char before @ and 3 chars after
    if(emailName.length < 1 || emailDomain.length < 3) {
        alert("Wrong number of characters before or after @ sign");
        return false;
    }

    // Domain must include but not start with .
    if(emailDomain.indexOf('.') <= 0) {
        alert("Domain must include but not start with .");
        return false;
    }

    // emailName must only include valid chars
    for (var i = emailName.length - 1; i >= 0; i--) {
        if(validChars.indexOf(emailName[i]) < 0) {
            alert("Invalid character in name section");
            return false;
        }
    };

    // emailDomain must only include valid chars
    for (var i = emailDomain.length - 1; i >= 0; i--) {
        if(validChars.indexOf(emailDomain[i]) < 0) {
            alert("Invalid character in domain section");
            return false;
        }
    };

    // Domain's last . should be 2 chars or more from the end
    if(emailDomainParts[emailDomainParts.length - 1].length < 2) {
        alert("Domain's last . should be 2 chars or more from the end");
        return false;   
    }

    alert("Email address seems valid");
    return true;
}
share|improve this answer
    
Thanks! This is excellent feedback :) Could you quickly explain the difference between return null and return false? –  GusRuss89 Dec 30 '13 at 4:29
    
@GusRuss89 You're asking "isValidEmail(email)" when passing in a value -- which you're either getting a YES (true) or NO (false). However, if you're not passing anything "isValidEmail()" you could return NULL since the function isn't technically answering isValidEmail() question. There's a number of advantages doing that, including improved debugging if you realize a specific parameter doesn't have a value passing through properly, or setting specific behavior if NULL is returned from isValidEmail. Always remember to watch out for type coercion ;) –  Rayan Bouajram Dec 30 '13 at 18:25

Hope I don't get blasted for this answer because its so easy, but since others provided a more detailed input I figured I would provide the "play the odds" answer.

This is the RegEx string that jQuery validate plugin uses for its email check:

/^((([a-z]|\d|[!#\$%&'\*\+\-\/=\?\^_`{\|}~]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])+(\.([a-z]|\d|[!#\$%&'\*\+\-\/=\?\^_`{\|}~]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])+)*)|((\x22)((((\x20|\x09)*(\x0d\x0a))?(\x20|\x09)+)?(([\x01-\x08\x0b\x0c\x0e-\x1f\x7f]|\x21|[\x23-\x5b]|[\x5d-\x7e]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(\\([\x01-\x09\x0b\x0c\x0d-\x7f]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF]))))*(((\x20|\x09)*(\x0d\x0a))?(\x20|\x09)+)?(\x22)))@((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)+(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))$/i

I rarely accept "its used by everyone so its a good idea" as a good answer, but I must admit that I use this RegEx (not the full plugin, just this RegEx string) in my code and its pretty bulletproof.

share|improve this answer

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.