0

I have written two objects. One is called List and another is called Car.

function Car()
{
  this.make="";
  this.year="";
}

function List()
{
  this.cars = [];
}

And I have another function, which is a method of object List to read in XML file's information, and store them in the car array inside List object:

List.prototype.readXML = function()
{
  var i = 0;
  $.get('mydata.xml', function(xml){
    $(xml).find("item").each(function(){
      //Bug Point!!!
      this.cars.push(new Car()); //Push a Car object into array
      this.cars[i].ID = ($(this).attr("ID"));
     }); 
   });
 }

However this won't work. Everytime the debug gives me that cars is not defined... I tried using var instead of this to define cars. And tried to remove this.cars.push instead of cars.push. But still it says the cars is not defined.

I assume that I may get something wrong with the variable scope in this problem. Could any one can teach me how to do it?

Thank you!

nnnnnn
  • 147,572
  • 30
  • 200
  • 241
Arthur0902
  • 209
  • 3
  • 14

3 Answers3

2

The problem is with your jquery each

$.get('mydata.xml', function(xml){
    $(xml).find("item").each(function(){
      //Bug Point!!!
      this.cars.push(new Car()); //Push a Car object into array
      this.cars[i].ID = ($(this).attr("ID"));
     }); 
   });

this doesn't refer to what you expect it to refer to

A common way of overcoming this problem is assigning it to a different variable called that or self

List.prototype.readXML = function()
{
  var i = 0;
  // Create a new variable called 'self' that you can refer to within different function scopes
  var self = this;
  $.get('mydata.xml', function(xml){
    $(xml).find("item").each(function(){
      self.cars.push(new Car()); //Push a Car object into array
      self.cars[i].ID = ($(this).attr("ID"));
     }); 
   });
 }

That means you have access to the original List object that you need to have access to. This method takes advantage of closures.

Hope this helps


Edit explaining question asked in comments:

List.prototype.readXML = function()
{
  // Create a new variable called 'self' that you can refer to within different function scopes
  var self = this;
  $.get('mydata.xml', function(xml){
    $(xml).find("item").each(function(i, domElem){
      var car = new Car();
      car.id = $(domElem).attr("ID");
      self.cars.push(car); //Push a Car object into array
     }); 
   });
 }
AlanFoster
  • 8,156
  • 5
  • 35
  • 52
  • Thank you @AlanFoster. I think that help. The self-this exchange is really a smart trick to overcome this problem. However I think I meet another bug that with this line of code: self.cars[i].ID = ($(this).attr("ID")); I put a Car object in the pervious line, so I suppose to access the members (which is ID) from this car[i].ID syntax, right? The debugger just told me "Uncaught TypeError: Cannot set property 'ID' of undefined " Could you help me? Thank you! – Arthur0902 Aug 20 '12 at 22:55
  • @Arthur0902 You could try to rewrite it like `function(i, xml) { var car = new Car(); car.id = $(xml).attr("ID"); self.cars.push(car); }` as the `this` in each is the value of i, and not the dom element – AlanFoster Aug 20 '12 at 23:02
  • @Arthur0902 I have modified my original question to include the source of what I am suggesting – AlanFoster Aug 20 '12 at 23:06
  • @ AlanFoster Thank you so much! Another brilliant solution! I really wanna to buy you a drink if I could : ) Thanks! – Arthur0902 Aug 20 '12 at 23:12
1

The problem you're having is related to closures.

Basically, the scope of this changes inside of your .each statement. The .each doesn't refer to List anymore, it refers to the current "item" inside of the XML.

To fix it, see How do JavaScript closures work?

List.prototype.readXML = function()
{
  var i = 0;
  var self = this;
  $.get('mydata.xml', function(xml){
    $(xml).find("item").each(function(){
      //Bug Point!!!
      self.cars.push(new Car()); //Push a Car object into array
      self.cars[i].ID = ($(this).attr("ID"));
     }); 
   });
}
Community
  • 1
  • 1
Luqmaan
  • 2,052
  • 27
  • 34
1

The problem is that the this context changes with the Ajax callback. I would define an addCar method on the prototype, and use that to add the new cars. Something like this:

List.prototype.addCar = function(data) {
  this.cars.push(new Car()); //Push a Car object into array
  this.cars[i].ID = ($(data).attr("ID"));
}

List.prototype.readXML = function()
{
  var i = 0;
  var self = this;
  $.get('mydata.xml', function(xml){
    $(xml).find("item").each(function(){ self.addCar(this); });
   });
 }
McGarnagle
  • 101,349
  • 31
  • 229
  • 260