-1

I am a JS newbi am trying to create an Array of objects. I have created the following object:

gameCard = new Object({
    image : "",
    container:"",
    match2Card:"",
    visible: false,
    cardId:"",

    updateCard: function (imageName,divName,matchingCard,isVisible,cardId) {
        this.image = imageName;
        this.container = divName;
        this.match2Card = matchingCard;
        this.visible = isVisible;
        this.cardId = cardId;
    },
    toggleCard: function (){
        if (this.visble) {
            this.visble = false;
        }
        else {
            this.visble = true;
        }
    },
    printCard : function() {
        document.write (this.image + ' ' + this.container + ' ' + this.match2Card + ' ' + this.visible + ' ' + this.cardId + '<br>') ;
        //alert (this.match2card);
    }
})

and run the following :

gameCards = new Array ();

for (i=0; i<20 ; i=i+1) {
    z=i+1;

    c1 = Math.floor((Math.random()*20)+1);

    gameCard.updateCard ( 'images/bf'+i+'.jpg' , 'div' + c1 , z , false,i) ;

    gameCards.push(gameCard);
}

when printing the array by using :

for (i=0;i<20; i++) {
    gameCards[i].printCard();
}

all items are the same.

what am I doing wrong ?

5
  • 3
    Use Object literal form {} instead of new Object() and same for Array [] instead of new Array() Commented Jun 7, 2013 at 12:21
  • 2
    Can it be you are updating the same instance all the time? Commented Jun 7, 2013 at 12:22
  • You're only creating one card and then adding it to the array over and over. You need to create multiple card instances inside your loop. Commented Jun 7, 2013 at 12:23
  • @PointedEars Yes, you're right with for loop it's just creating 20 pointer for same object Commented Jun 7, 2013 at 12:25
  • 1
    @Givi Afterwards the array contains 20 references to the same object. Commented Jun 7, 2013 at 12:27

2 Answers 2

1

You are using only one object, you must use

gameCard = {…} in the loop. (new Object is not necessary)

for (var i = 0; i < 20; ++i) {
   var gameCard = {…}; // create a new instance of object
   …
}
Sign up to request clarification or add additional context in comments.

15 Comments

The Object constructor does not use its argument for initialization; it converts it into an object. Therefore the Object constructor is unnecessary here; the object initialiser {…} already creates an Object instance and evaluates to a reference to that instance.
@PointedEars Yes, but the important is to create a new instance in the loop
@PointedEars what? this answer looks right to me. If there's a further mistake the OP made, this answer covers the most important one.
@djechlin It looks right now. You are falsely assuming that I downvoted it.
@PointedEars why does it look right now? The var decl didn't matter.
|
1

Either define your GameCard object like so (fiddle):

var GameCard = function () { //wrap in a function to mimic a constructor
    return {
        "image": "",
        "container": "",
        "match2Card": "",
        "visible": false,
        "cardId": "",
        "updateCard": function (imageName, divName, matchingCard, isVisible, cardId) {
            this.image = imageName;
            this.container = divName;
            this.match2Card = matchingCard;
            this.visible = isVisible;
            this.cardId = cardId;
        },
        "toggleCard": function () {
            this.visible = !this.visible;
        },
        "printCard": function () {
            log(this.image + ' ' + this.container + ' ' + this.match2Card + ' ' + this.visible + ' ' + this.cardId + '<br>');
        }
    };
};

var test = function () {
    var gameCards = [],
        card = null,
        i = 0;
    for (i = 0; i < 20; i = i + 1) {
        // call the function to get a new object
        card = GameCard();
        // set object properties
        z = i + 1;
        c1 = Math.floor((Math.random() * 20) + 1);
        card.updateCard('images/bf' + i + '.jpg', 'div' + c1, z, false, i);
        // push into array
        gameCards.push(card);
    }
    for (i = 0; i < 20; i++) {
        gameCards[i].printCard();
    }
};
test();

or define it like this (fiddle):

var GameCard = function () { //or make a constructable object
    this.image = "";
    this.container = "";
    this.match2Card = "";
    this.visible = false;
    this.cardId = "";
    this.updateCard = function (imageName, divName, matchingCard, isVisible, cardId) {
        this.image = imageName;
        this.container = divName;
        this.match2Card = matchingCard;
        this.visible = isVisible;
        this.cardId = cardId;
    };
    this.toggleCard = function () {
        this.visible = !this.visible;
    };
    this.printCard = function () {
        log(this.image + ' ' + this.container + ' ' + this.match2Card + ' ' + this.visible + ' ' + this.cardId + '<br>');
    };
};

var test = function () {
    var gameCards = [],
        card = null,
        i = 0;
    for (i = 0; i < 20; i = i + 1) {
        // instantiate new object
        card = new GameCard();
        // set properties
        z = i + 1;
        c1 = Math.floor((Math.random() * 20) + 1);
        card.updateCard('images/bf' + i + '.jpg', 'div' + c1, z, false, i);
        // push into array
        gameCards.push(card);
    }
    for (i = 0; i < 20; i++) {
        gameCards[i].printCard();
    }
};
test();

Otherwise, what you're doing is pushing the same instance into the array 20 times.

UPDATE

You could also define your GameCard object as you're currently doing and then call Object.create(GameCard) to create a new instance of it (fiddle):

var GameCard = {
    "image": "",
    "container": "",
    "match2Card": "",
    "visible": false,
    "cardId": "",
    "updateCard": function (imageName, divName, matchingCard, isVisible, cardId) {
        this.image = imageName;
        this.container = divName;
        this.match2Card = matchingCard;
        this.visible = isVisible;
        this.cardId = cardId;
    },
    "toggleCard": function () {
        this.visible = !this.visible;
    },
    "printCard": function () {
        log(this.image + ' ' + this.container + ' ' + this.match2Card + ' ' + this.visible + ' ' + this.cardId + '<br>');
    };

var test = function () {
    var gameCards = [],
        card = null,
        i = 0;
    for (i = 0; i < 20; i = i + 1) {
        // use Object.create for a constructor
        card = Object.create(GameCard);
        // set properties
        z = i + 1;
        c1 = Math.floor((Math.random() * 20) + 1);
        card.updateCard('images/bf' + i + '.jpg', 'div' + c1, z, false, i);
        // push into array
        gameCards.push(card);
    }
    for (i = 0; i < 20; i++) {
        gameCards[i].printCard();
    }
};
test();

8 Comments

Using a function as constructor that does not have prototype methods or properties to return an object is needlessly complicated, except when you use closures (have “private properties” in the constructor).
thx you all for you help am going to check what you suggested and will update if I was able to solve this issue.
am still getting the same result :( I wanted create an Array that will include 20 items of the object gameCard (should have beed 20 different item and sill getting the same items 20 items. I have tried to use gamesCards[i] = new object (gameCard); but it did not help
@user2463457: I minimized the code needed for the answer. Check out the included (and working) jsFiddles and compare those with your code.
when running the updated fiddle am getting the same issue that am currently handling all the Items in the Array are having the same value.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.