1

The variable Message and boolean value count from function validator won't get updated when it runs inputFocus function. So, validator always returns true.

(function(){
    var form = document.forms[0];
    form.addEventListener('submit', function(evt){
        if(!validator(form)){
            evt.preventDefault();
        }
    }, false);      
})();

main.js

function validator(form){
    var message = "The input below is invalid";
    var count = true;
    if(!phoneValidation(form.phone.value)){
        inputFocus(form['phone'], "focus", message, form.phone.name, count);
    } 

    if(!count){
        alert(message);
        return count;
    } else {
        return true;
    }
}

function inputFocus(element, newClassName, message, id, count){
    element.className += " " + newClassName;
    message += ", " + id;   
    if (count){
        count = false;
    }
}
Chris
  • 26,544
  • 5
  • 58
  • 71
Tel
  • 13
  • 2
  • The call inside validator passes the value of count to inputFocus "by value" and not "by reference" which means that the two count variables refer to different things. –  Mar 11 '14 at 06:00

3 Answers3

0
function inputFocus(element, newClassName, message, id, count){
    element.className += " " + newClassName;
    message += ", " + id;   
    if (count){
        count = false;
    }
}

So when you pass the parameters (including message and count) to inputFocus, given the fact that they are scalars (a boolean and a string), then whatever inputFocus does with the variables local to it "stays" within inputFocus. This is because it's passed the values of the scalars, not the variables per se.

In short:

inputFocus doesn't change whatever variables whose values happened to be passed to it.

One potential way to solve this is to have inputFocus return the correct values of count and message, then validator can do as it wishes with those values.

Hope this gets you on the right track.

Chris
  • 26,544
  • 5
  • 58
  • 71
0

Move the variables Message and count outside all the functions to make it like global. This will let any javascript function in the page to update them

var message;
var count;
function validator(form){
    message = "The input below is invalid";
    count = true;
    if(!phoneValidation(form.phone.value)){
        inputFocus(form['phone'], "focus", message, form.phone.name, count);
    } 

    if(!count){
        alert(message);
        return count;
    } else{
        return true;
    }
}

function inputFocus(element, newClassName, message, id, count){
    element.className += " " + newClassName;
    message += ", " + id;   
    if (count){
        count = false;
    }
}

At the same time its not a good practice to use global variables if you can merge the functions that uses these variables into one.

A J
  • 2,112
  • 15
  • 24
  • This is bad practice. It voids the whole point of having two separate functions. – Chris Mar 11 '14 at 06:09
  • @Chris: wanted to point the fact that these variables are local to the functions and if they are to be accessed by multiple functions declare them globally. – A J Mar 11 '14 at 06:11
  • Agreed. Upvoted to counteract. :) My answer doesn't pollute the global scope as much but uses the same principle - and they both serve to illustrate the principle of variable scope the OP is missing. – Will Mar 11 '14 at 06:13
  • 1
    If they are to be accessed by multiple functions, then the multiple functions should either be merged or you have another design issue. JavaScript is asynchronous by design: having potential for simultaneous access of a set of variables in a scope is bad practice. Please don't promote it. – Chris Mar 11 '14 at 06:14
  • @Will Having an object wrapper around them only solves a tiny insignificant fraction of the issue. The issue is an issue of scope and access semantics, not merely polluting the global scope. – Chris Mar 11 '14 at 06:16
  • 1
    @Chris : Thank you for passing the information .. so do you think I should delete this answer? or may be I shall add your comments to the answer. – A J Mar 11 '14 at 06:18
  • @AJ It is completely up to you. Per [StackOverflow answer guidelines](http://stackoverflow.com/questions/how-to-answer), your answer is perfectly fine. I was merely expressing a technical, content-oriented perspective in my comments above. Whether it's helpful or not is up to the community to decide — **you** get to decide whether you believe keeping it will be helpful. – Chris Mar 11 '14 at 06:22
  • Cheers for the explanation @chris. Not sure I agree with you/understand you though. If create a `car = {}` and the `car` has a `car.fuelLevel` property I would adjust that property from both the `car.driving()` method and the `car.refuel` method. I'm assuming you wouldn't advocate combining those methods but perhaps I'm misunderstanding. I answer questions here occasionally mostly because it makes me learn stuff so I'm not arguing, I'm looking to understand. – Will Mar 11 '14 at 06:30
  • @Will If `car.driving` called `car.refuel` (which is where the analogy stops being... analogous), I would indeed advocate combining those methods. – Chris Mar 11 '14 at 06:35
  • Think what I'm getting at is that `car.fuelLevel` being adjusted by `car.driving` is the same as `window.myApp` be adjusted by `window.myfunc`. But ok. – Will Mar 11 '14 at 06:38
  • @Will I understand. However, in your `car` example, the two functions are completely independent. In the OP's program, one function explicitly depends on the other function — `inputFocus` only gets called by `validator`. – Chris Mar 11 '14 at 06:40
-1

That's because when you set the value of message and count within the inputFocus function, the value of those variables are only available within the scope of that function.

Here's an evening's worth of reading. Absolutely worth it if you want to write javascript.

  1. What is the scope of variables in JavaScript?
  2. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions_and_function_scope
  3. http://coding.smashingmagazine.com/2009/08/01/what-you-need-to-know-about-javascript-scope/
  4. http://blog.kevinchisholm.com/javascript/scope/
Community
  • 1
  • 1
Will
  • 4,075
  • 1
  • 17
  • 28