CODEWITHSUNDEEP
javascriptfunctionparametersthis
varrtto
varrtto

Reputation: 101

Javascript 'this' as parameter

I'm trying to copy the value from "FROM" fields into "TO" fields. My first attempt was this:

function updateToField(toField,fromField)
{
    toField.value = fromField.value}
}

function verifyFromToFields()
{
var inputs = getElementsByTagName("input");
for (var j = 0; j < inputs.length; j++)
{
    if (inputs[j].name.indexOf('FROM') != -1 && if (inputs[j+1].name.indexOf('TO') != -1)
        {
            var fromField = inputs[j];
            var toField   = inputs[j+1];
            fromField.onchange = function(){updateToField(toField,fromField)};
        }
}

The website has several FROM-TO pairs, and this only seem to work for the last pair in the "inputs" array.

Then I tried this:

function updateToField(toField,fromField)
{
    toField.value = fromField.value}
}

function verifyFromToFields()
{
var inputs = getElementsByTagName("input");
for (var j = 0; j < inputs.length; j++)
{
    if (inputs[j].name.indexOf('FROM') != -1 && if (inputs[j+1].name.indexOf('TO') != -1)
        {
            var fromField = inputs[j];
            var toField   = inputs[j+1];
            fromField.onchange = function(){updateToField(toField,this)};
        }
 }

With this, when any FROM field in the page is modified, it's copied to the last TO field on the page. I think this is one of those issues I've read about parameters as value or reference, but I can't figure it out. Also this is a VERY simplified version of the code, I actually fill the inputs list with a getElementsByClass function and must search through childnodes. Does anyone have a clue on whats going on?

Upvotes: 3

Views: 26118

Answers (3)

Michael Lorton
Michael Lorton

Reputation: 44386

That closure, I do not think it means what you think it means.

This line here:

     fromField.onchange = function(){updateToField(toField,this)};

means "assign to onchange a function that assigns the contents of that fields to whatever toField is at the time of the change!

Since you only have one variable toField all the changeable fields will be assigned to it.

This would work:

var setOnChange = function(fromField, toField) {    
   fromField.onchange = function(){updateToField(toField,this)};
};

for (var j = 0; j < inputs.length; j++)
{
    if (inputs[j].name.indexOf('FROM') != -1 && if (inputs[j+1].name.indexOf('TO') != -1)
        {
            setOnChange(inputs[j], inputs[j+1]);
        }
 }

EDIT: Isaac might have a better explanation of the problem (although I don't really like his solution).

Upvotes: 1

user24359
user24359

Reputation:

The problem actually has to do with scope. What's happening is that your functions (the ones you assigned to onchange) are capturing the variables toField and fromField and their values keep changing. I know it looks like you've declared them anew each time through the loop, but that's not how JS works; consecutive trips through a loop share a scope, so fromField is the same variable each time through and you're merely assigning it a new value in each iteration. So at the end, all of your functions refer to the same fromField variable. And that fromField variable, naturally, contains the last value you assigned it.

So when you eventually call all of those functions, they all do the same thing, because all of their fromFields (and, by the same logic, toFields) are the same variable. So that explains why only the last inputs worked; they're what fromField and toField contained when you ran the functions.

You can fix this by introducing an intermediate function, since functions do create new scopes. That way each time through the loop, you get brand new variables.

function updateToField(toField,fromField)
{
   toField.value = fromField.value;
}

function verifyFromToFields()
{
  var inputs = getElementsByTagName("input");
  for (var j = 0; j < inputs.length; j++)
  {
    function(){
      if (inputs[j].name.indexOf('FROM') != -1 && if (inputs[j+1].name.indexOf('TO') != -1)
      {
          var fromField = inputs[j];
          var toField   = inputs[j+1];
          fromField.onchange = function(){updateToField(toField,fromField)};
      }
   }();
}

Upvotes: 1

Lucas Gabriel S&#225;nchez
Lucas Gabriel S&#225;nchez

Reputation: 41600

That's is because of how closures work. When you assign the onchange function you create (for every loop) a new anonymous function that calls updateToField, but the value of the toField and this parameters (or any other you are passing) are not bind to the "current" loop, instead that parameters are bind to the last value of the loop (that's why is only working with your last "TO").

Instead of assigning a new function to onchange property try calling the Function.bind if you're running in an environment that has that or write one if you don't have it.

Here are the documentation for bind

So you can go like this:

fromField.onchange = updateToField.bind(this, fromField, toField);

Or you can use the other approach that Malvolio wrote.

Upvotes: 1

Related Questions