Barney
Barney

Reputation: 1848

JS function returning false variable that should be true

I'm having an issue with the state of a returned variable. I have the function "altFormValidate". Essentially the issue is that when I call it, if the honeypot count is 0, the validation runs fine, and the "validationResult" variable returns true, but if the honeypot count is more than 0, the "validationResult" variable still returns false, even after executing code that should make it true.

function altFormValidate(intendedFormID){ //Validate honeypot traps
    var validationResult = false;
    }

    var honeyPotFields = ["Fname", "EmailAddress", "Lname", "Telephone", "Address1", "Address2", "Surname", "Title"];
    var honeyPotCount = 0;
    honeyPotFields.forEach(function(entry) { //Check page for honeypots filled out
        if($('#' + entry).length >= 1){
            var honeyPotVal = $('#' + entry).val();
            if (honeyPotVal !== ""){
                honeyPotCount = honeyPotCount + 1; //Count successful honeypot traps
            }
        }
    });
    if (!honeyPotCount > 0) //Check honeypot count
    {
        $('#' + intendedFormID).attr("action", formAction); //Append SF action to form
        validationResult = true;
    }
    else 
    {
        if (honeyPotCount == 1){ //If one honeypot filled in
            //SUSPECT - implement further checks
                $.colorbox({ //init popup with confirmation input: "#machine-check"
                href: "/inc/form-confirmation.html",
                transition: "elastic",
                speed: 350,
                opacity: 0.65,
                className: "seg-window",
                onComplete: function() {
                    jQuery('#colorbox').find('a.btn-close, a.close, a.cancel').unbind('click.closeColorbox').bind('click.closeColorbox', function(e) {
                        jQuery.colorbox.close();
                        e.preventDefault();
                    });
                    jQuery('#machine-check').bind('change', function(e) {
                            jQuery.colorbox.close();
                            e.preventDefault();
                            validationResult = true; //This code doesn't seem to change the variable "validationResult" when I call the entire function altFormValidate but I know that changing "machine-check" does change the variable. 
                        });
                    }
             });
        }
        else if (honeyPotCount == 2){ //If two honeypots filled in
            //PRIME SUSPECT - init Captcha/Akismet etc
            alert("Robot Alert!");
            validationResult = false;
        }

    } return validationResult;
}

and I call above function like so, on submit of the form:

   var req = true; 
   if(req){ //If required fields are filled in
                var extraValidation = altFormValidate('WebToLeadForm', 'webToLeadPost');

                 if (extraValidation == true){
                    alert("form sent");
                    //document.WebToLeadForm.submit();
                    return true;
                }
            }

So my question is why does extraValidation() return true if no honeypots are found, but false if one is found but the relevant code is then executed to make it true? I am thinking the issue is where I return the validationResult variable.

Upvotes: -1

Views: 1442

Answers (2)

Useless Code
Useless Code

Reputation: 12422

if (!honeyPotCount > 0) does not do what you think it does. It does the opposite.

If honeyPotCount is 0 it will return true, otherwise it will return false.

Logical-not ! has a higher precedence than greater than >. So here's what actually happens in your if statement:

  • It takes honeyPotCount and coerces it to a Boolean value and then returns it's inverse.
    • !0 returns true
    • !-1, !1, !2, etc return false.
  • That Boolean value is then corersed to a number so it can be compared to 0.
    • So, if honeyPostCount === 0, !honeyPotCount === true, true == 1 and 1 > 0 === true.
    • But, if honeyPostCount === 1, !honeyPotCount === false, false == 0 and 0 > 0 === false.

You could fix it by adding parentheses to override the precedence if (!(honeyPotCount > 0)) meaning "if honeyPotCount is not greater than 0". But it would be more readable to remove the negation from the logic and change it to if (honeyPotCount < 1), meaning "if honeyPotCount is less than 1". Even better, you never decrement honeyPotCount, so you could also make it even simpler and clearer by changing it to if (honeyPotCount === 0).

Upvotes: 1

Rob
Rob

Reputation: 11798

I didn't test it, but I guess that

if (!honeyPotCount > 0)

is not the same as

if (!(honeyPotCount > 0))

Upvotes: 2

Related Questions