Javascript issue. variable set within function but outside of function it isn't

Question

I have some code here

var F;
var favv= ['E','I','A','O','U'];
var i = 0;

function vowelcount(arg, favv)
{
  for(i=0;i<favv.length;i++) {
    c = 0;
    V = favv[i];
    for (j=0;j<arg.length;j++) {
      if (arg[j].toUpperCase()===V) {
        c++;
      };    
    }
    if (c>0) {
      F=V;
      return c;
    }
  }
}

var person1 = {name:"Super",spd:20};
var person2 = {name:"Supeer",spd:20};

function Scheck(person1, person2) {

  if (person2.spd>person1.spd) {

    var sub=person1;
    person1=person2;
    person2=sub;

  } else if (person2.spd===person1.spd) {

    var ct1 = vowelcount(person1.name, favv);
    var ct2 = vowelcount(person2.name,F);

    if (ct2 > ct1) {  
      var subp = person1;
      person1= person2;
      person2=subp;
    }
  }
  console.log(person1);
  console.log(person2);
}

Scheck(person1,person2);

console.log(person1);
console.log(person2);

Here I have an Array of vowels, two people with properties name and spd. When I run Scheck, I want to use vowelcount to determine the order that people would move if their Speed stats are equal. If you look at the console.logs inside the function, they print the correct names... but after the function the console.logs print the original order. Why is this?

Possible duplicate: http://stackoverflow.com/questions/500431/javascript-variable-scope — ernie, Jan 13 '14 at 17:45
lol lukasz.. haha, it probably is. I want to rewrite a lot of stuff. — Chris Jones, Jan 13 '14 at 17:51
I fixed up your code so us poor souls could understand it better... Try to make your code presentable before posting it on SO. It will get you better and faster responses. — dthree, Jan 13 '14 at 18:05

score 4 · Answer 1 · answered Jan 13 '14 at 17:45

4

You have a scope issue. You declare person1 and person2 on the global scope, but then make them your function parameters, which is sloppy.

Name the parameters in your function something different, and then maek Scheck actually return values for person1 and person2.

You could make Scheck like this:

function Scheck(p1, p2)
{
  // ... do things

  return [p1, p2];        
}

And then receive it like this:

var persons = Scheck(person1, person2);
person1 = persons[0];
person2 = persons[1];

answered Jan 13 '14 at 17:45

dthree

19,847
14
77
106

I think I actually figured it out kind of, but I do agree.. that is somewhat sloppy on my part. since I have already declared person1, person2, at least in this instance, I don't really need to use them as parameters, at least that is my understanding anyways. – Chris Jones Jan 13 '14 at 17:49
Yes, exactly. You should never call a function this way. – dthree Jan 13 '14 at 18:07

score 2 · Answer 2 · answered Jan 13 '14 at 17:45

2

You're assigning the function's parameters (which create local variables), not the global variables.

If you want to mutate global state, you shouldn't have those parameters at all.

answered Jan 13 '14 at 17:45

SLaks

868,454
176
1,908
1,964

score 2 · Accepted Answer · answered Jan 13 '14 at 17:52

var subp = person1;
person1= person2;
person2=subp;

This swaps round the values of the two arguments person1 and person2, but they are local to the function, so it does not affect the variables outside the function.

One easy solution in your above code is to use a closure. Don't pass the variables in, or receive them!

function Scheck() {

...

Scheck();

Now inside the Scheck function, since there are no local variables or arguments called person1 and person2, it will use the variables from the outer scope. Then the swap will work as intented.

Another option would be to pass an object or an array to the Scheck function. The function will them be able to change the properties of the object, and these changes will be reflected outside, because the same object is being referenced. You could use an array, but here is the example with an object:

function Scheck(people) {

    var person1 = people.first;    // or people[0] if you used an array
    var person2 = people.second;   // or people[1] if you used an array

    ...

            if(ct2>ct1)
                {  

                    people.first = person2;   // Set people[0] for an array
                    people.second = person1;

                }

...

var people = {
    first: person1,
    second: person2
};
Scheck();
person1 = people.first;
person2 = people.second;

// or for an array:
var people = [person1, person2];
Scheck(people);
person1 = people[0];
person2 = people[1];

I actually ended up doing your first option. I just didn't pass it any parameters. Glad to learn some things today though. — Chris Jones, Jan 13 '14 at 17:53

score 1 · Answer 4 · answered Jan 13 '14 at 17:49

1

In your Scheck, you use F as the second parameter. F has been declared but not given a value so it fails as num > undefined is false and nothing changes.

answered Jan 13 '14 at 17:49

robotmayo

131
10

Actually, I've not noticed any issue with F myself considering that I only need to run this function twice at the moment. – Chris Jones Jan 13 '14 at 17:50
I just noticed that in vowelcheck, it sets F to the vowel so it becomes a string which has a length property so it runs fine because you run it with favv before you run it with F. – robotmayo Jan 13 '14 at 17:56
Well spotted. Since it is only used locally, `F` should probably be declared `var F` at the top of that function, not at the top of the file! And also give it a name... ;) – joeytwiddle Jan 13 '14 at 18:00
I could be wrong, but I think it is more than just local since it is used in two functions. – Chris Jones Jan 13 '14 at 18:05

Javascript issue. variable set within function but outside of function it isn't

4 Answers4