Using Object.create in a forEach loop

Question

I am writing a card game using javascript/html5

I get the gamestate as an ajax request. this is JSON data that lists the players and what cards they have in their hand

I am trying to loop over each player and set the hand data as follows

gameState.Players.forEach(function (player, i) {    
    var inx = i + 1;
    var canvas = document.getElementById("player" + inx);
    var ctx = canvas.getContext("2d");

    var hand = Object.create(Hand);
    hand.initialiseHand(player.Hand);
    hand.setPosition(10, 10);
    hand.draw(ctx);
});

There are six canvases on the page. One for each player

I am using Object.create to create a new "instance" of a hand. And then calling the draw method, which lays out the images on the canvas

However, what actually happens is that each players data just gets added to the same instance

i.e. each time I go round the forEach loop, the hand object just gets assigned more and more cards

I would like to have a seperate instance for each player

So how do I achieve this?

I want to loop over the data and create a new hand for each iteration of the loop

I am guessing that the hand variable has been hoisted out of the loop and so I get the same one each time?

and this is what Hand looks like

var Hand = {

    faceDownCards: [],
    faceUpCards: [],
    inHandCards: [],

    initialiseHand: function (handData) {
        handData.FaceDownCards.forEach(function (c, i) {
            this.faceDownCards.push(Object.create(Card, pd({ rank: c.Rank, suit: c.Suit })));
        }, this);

        handData.FaceUpCards.forEach(function (c, i) {
            this.faceUpCards.push(Object.create(Card, pd({ rank: c.Rank, suit: c.Suit })));
        }, this);

        handData.InHandCards.forEach(function (c, i) {
            this.inHandCards.push(Object.create(Card, pd({ rank: c.Rank, suit: c.Suit })));
        }, this);

    },

    draw: function (context) {
        this.faceDownCards.forEach(function (c, i) {
            c.draw(context);
        });

        this.faceUpCards.forEach(function (c, i) {
            c.draw(context);
        });

        this.inHandCards.forEach(function (c, i) {
            c.draw(context);
        });
    },

    setPosition: function (x, y) {
        this.x = x;
        this.y = y;
        this.faceDownCards.forEach(function (c, i) {
            c.setPosition(x + i * 70, y);
        });
        this.faceUpCards.forEach(function (c, i) {
            c.setPosition(x + i * 70, y + 60);
        });
        this.inHandCards.forEach(function (c, i) {
            c.setPosition(x + i * 20, y + 80);
            //c.rotation = 3;
        });
    }
};

*"I am guessing that the hand variable has been hoisted out of the loop and so I get the same one each time?"*: No, because each iteration invokes the callback you pass. — Felix Kling, Sep 11 '11 at 17:07
You could try explicitly adding the third argument to your callback. See https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/forEach. But, I don't think there is a problem with your forEach() loop. Maybe there is something in the Hand code. — adamleerich, Sep 11 '11 at 17:25
Why are you using `Object.create(Hand)` instead of `new Hand`? This answer might help you: http://stackoverflow.com/questions/4166616/understanding-the-difference-between-object-create-and-new-somefunction-in-ja — Jordan Running, Sep 11 '11 at 17:26

hugomg · Accepted Answer · 2011-09-11T18:23:49.757

Your hand variable is not being hoisted out of the loop. Javascript variables have function scope and you are already conveniently using a function inside your forEach loop.

What is Hand? The usual convention is having capitalized names represent constructor functions, but the Players and the Object.create make it look like Hand is just an object?

If Hand is already an object (and not a Constructor you are misusing) my best bet would be that initializeHand/setPosition are seting closed-over variables instead of accessing them via this. (Of course, this is just a wild guess without looking at the Hand code)

After looking at your Hand code, I now think the problem is that the hands are sharing the faceDownCards, etc, arrays. The base prototype should only be used for shared traits and the arrays and other instance-olny state should be set on initialization :

For a concrete example, change

handData.FaceDownCards.forEach(function (c, i) {
    this.faceDownCards.push(Object.create(Card, pd({ rank: c.Rank, suit: c.Suit })));
}, this);

to

 this.faceDownCards = handData.FaceDownCards.map(function (c, i) {
     return Object.create(Card, pd({ rank: c.Rank, suit: c.Suit }));
 });

ps.: All the Object.create going around is not exactly idiomatic Javascript. But then, I guess you should have an Idea of what you are doing, right?

perfect - thanks a lot, that works great. I am thinking I should remove the initialiseHand function, and instead pass the data in on Object.create()'s second parameter - what do you think? — ChrisCa, Sep 11 '11 at 18:30
I am just getting my head around OO javascript so can well imagine this is not idiomatic. Can you tell me a better approach. I am doing this to get to know HTML5 and javascript a bit better so any pointers are very welcome! — ChrisCa, Sep 11 '11 at 18:33
The "normal" javascript way would be putting the initialization in a constructor function, the traits on the ".prototype" property of this function and then build stuff via `new` syntax. This is actually a big hack but it was actually the onlt way to do prototypes ebfore Object.create came around. In your case, if you can get off by passing the data directly then do as you wish. If things are a little more complicated having an init function is perfectly fine though. — hugomg, Sep 11 '11 at 19:05

Using Object.create in a forEach loop

1 Answers1