1
\$\begingroup\$

I've made a few functions to check form input for an AJAX request. I am still getting used to JavaScript and some of these AJAX requests. I am looking for any suggestions to better work with JavaScript functions, and passing variables through a function level scoping.

//Form Handler
$("#form-submit").submit(function(e) {
    e.preventDefault(); 
    $.ajax({    
        beforeSend: function(){ 
            return (checkAll());
            return (passwordCheck());
        },
        complete: function(){
        },
           type: "POST",
           url: '',
           data: $("#form-submit").serialize(),
           success: function(data)
           {
           //window.location = '/dashboard.php';
           // Set Session Php Etc 
           alert('php ran');
           return true;
           }
         });
}); 

//Pushing all Input Values to an Array  
function checkAll() {
    var arr = [];
    $('#form-submit :input').each(function() {
        arr.push($(this).val());
    });
    return (checkArray(arr));   
}

//Checks Array for empty strings 
function checkArray(arr){
    for(var i=0; i < arr.length; i++) {
        console.log(arr[i]);
        if (arr[i].trim() == '') {
            alert('Please Enter All Fields');   
            return false;
        } else {
            return true;
        }   
    }
}

//Matches Password
function passwordCheck() {
    var pass1 = $('#password').val();
    var pass2 = $('#password-check').val();
    if(pass1 != pass2) {
        $('#password').addClass('highlight');
        $('#password-check').addClass('highlight');
        alert("Passwords don't match");
        return false;
    } else {
        $('#password').removeClass('highlight');
        $('#password-check').removeClass('highlight');  
        return true;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ In the beforeSend, you have two return statements. \$\endgroup\$ – hjpotter92 Sep 6 '14 at 8:35
  • \$\begingroup\$ Is that not ideal? \$\endgroup\$ – miasyntax Sep 6 '14 at 8:41
  • \$\begingroup\$ You can return only once from your function call. \$\endgroup\$ – hjpotter92 Sep 6 '14 at 8:42
  • \$\begingroup\$ How would you go about checking an ajax based off of two Boolean based function? is there a method of concatenating function returns? Or is my methodology off? \$\endgroup\$ – miasyntax Sep 6 '14 at 8:45
  • \$\begingroup\$ The methodology is wrong. Use return (passwordCheck() && checkAll());? \$\endgroup\$ – hjpotter92 Sep 6 '14 at 8:46
5
\$\begingroup\$

Bugs

You may think this code works, but it probably doesn't.

Example 1:

beforeSend: function () {
    return (checkAll());
    return (passwordCheck());
},

The passwordCheck function will never be called. Most probably you wanted this:

return checkAll() && passwordCheck();

Example 2:

for (var i = 0; i < arr.length; i++) {
    console.log(arr[i]);
    if (arr[i].trim() == '') {
        alert('Please Enter All Fields');
        return false;
    } else {
        return true;
    }
}

This for statement doesn't loop: it returns after checking the first value in arr, it won't check the others. Most probably you wanted this:

for (var i = 0; i < arr.length; i++) {
    console.log(arr[i]);
    if (arr[i].trim() == '') {
        alert('Please Enter All Fields');
        return false;
    }
}
return true;

Also, a sligthly simpler way to check for blank value:

if (!arr[i].trim()) {

Efficiency

In passwordCheck you lookup $('#password') and $('#password') twice. It would be better to lookup only once and cache in a local variable, for example:

function passwordCheck() {
    var $password = $('#password');
    var $password_check = $('#password-check');
    var pass1 = $password.val();
    var pass2 = $password_check.val();
    if (pass1 != pass2) {
        $password.addClass('highlight');
        $password_check.addClass('highlight');
        alert("Passwords don't match");
        return false;
    } else {
        $password.removeClass('highlight');
        $password_check.removeClass('highlight');
        return true;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ After testing it, I found exactly what you were referring to it not calling the check password function. I reworked it so that it would return undefined, and ran it as return (passwordCheck() && checkAll()); But this in an of itself was a dirty fix. I didn't think of moving the return outside of the for loops. Thank You, I am still pretty new to programming this was very helpful. \$\endgroup\$ – miasyntax Sep 6 '14 at 19:08

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.