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.

Would like some assistance in improving my jQuery validation code. I have some repeated code, but I am not sure how to write it in a way so that I don't have to keep repeating the same code over and over again for different variable names etc... (such as the if statement part) My goal is to make it so that I can easily apply it to any other form I create.

Here is the jsFiddle

Javascript:

(function($) {

    $('.commentForm').submit(function () {

        var error = 'error';

        var name = $(this).find('.name')
        nameVal = name.attr('value')
        email = $(this).find('.email')
        comment = $(this).find('textarea')
        commentVal = comment.html();

        if (!(name.val() === nameVal || name.val() === '' || name.val().length < 3) && 
            !(comment.val() === commentVal || comment.val() === '' || comment.val().length < 3) && 
            validateEmail(email.val())
           ) {
            console.log('Form is good');
            return true;
        } else {

            (name.val() === nameVal || name.val() === '' || name.val().length < 3) ? name.addClass(error) : name.removeClass(error);
            (comment.val() === commentVal || comment.val() === '' || comment.val().length < 3) ? comment.addClass(error) : comment.removeClass(error);
            (!validateEmail(email.val())) ? email.addClass(error) : email.removeClass(error);

            console.log('Form is BAD');
            return false;

        }

    });

    function validateEmail(email) {
        var re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
        return re.test(email);
    }

}) (jQuery);

Thanks for your help in advance!

UPDATE CODE: (SAME FUNCTIONALITY)

$('form.requiredFields').submit(function(e) {
    var req = $(this).find('.req'),
        validateEmail = function(email) {
            var re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
            return re.test(email);
        };

    req.each(function() {
        var $this = $(this),
            defaultVal = $this.prop('defaultValue');

        if ( ( $this.hasClass('email') && !validateEmail( $this.val() ) ) || (defaultVal === $this.val() || $this.val() === '' || $this.val().length < 3) ) {
            $this.addClass('_error')
        } else {
            $this.removeClass('_error req');
        }


    });

    console.log(req.length);
    if ( req.length === 0 ) {
        return true;
    } else {
        return false;
    }

});
share|improve this question
    
If you can use plugin, jquery validation plugin comes with email validation. If not, you can have a look at their implementation. You can find it here: validation.bassistance.de/email-method –  user27584 Jul 23 '13 at 2:51
    
Likely moot at this point, but you can remove the check for $this.val() === '' as it is true if $this.val().length < 3 is true. –  Daniel Cook Nov 20 '13 at 14:24
add comment

1 Answer

$('.commentForm').submit(function() {
        var error = 'error',
            name = $(this).find('.name'),
            nameVal = name.attr('value'),
            email = $(this).find('.email'),
            comment = $(this).find('textarea'),
            commentVal = comment.html(),
            validElements = [],
            inValidElements = [];


        if( name.val() === nameVal || name.val() === '' || name.val().length < 3 ){
            inValidElements.push(name);
        }else{
             validElements.push(name);
        }

        if(comment.val() === commentVal || comment.val() === '' || comment.val().length < 3){
            inValidElements.push(comment);
        }else{
            validElements.push(comment);
        }

        if(validateEmail(email.val())){
            validElements.push(email);
        }else{
            inValidElements.push(email);
        }

        validElements.forEach(function(elem){elem.removeClass('error')});
        inValidElements.forEach(function(elem){elem.addClass('error')});

        if (inValidElements.length===0) {
            console.log('Form is good');
            return true;
        }
        console.log('Form is BAD');
        return false;
    });

    function validateEmail(email) { 
        var re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
        return re.test(email);
    }  

http://jsfiddle.net/awan/9gZhc/8/

I just tried to remove duplicate code. Hope You will improve it.

share|improve this answer
    
Thanks. I actually improved the code a lot and also shortened it tremendously. Although I do notice a few minor bugs that need to be fixed. –  Shivam Bhalla Jul 22 '13 at 19:44
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.