0

The code below is supposed to add an event listener for each playing card in a players hand while it's their turn, and then remove the events while it's a different player's turn.

It isn't working. That player's cards remain clickable once the event is initially set on their first turn.

takeTurn ( playerIndex ) {
    console.log(this.name);
    let handCards = document.getElementById(this.name).querySelector('.handCards');
    let theCards = handCards.querySelectorAll('.handCards .card');
    let that = this;
    for ( let h = 0; h < theCards.length; h++ ) {
        theCards[h].addEventListener("click", function onPlayCard (){
            let theseCards = handCards.querySelectorAll('.handCards .card');
            let discarded = that.playCard(theCards[h], theseCards, handCards);

            that.gameInstance.discardPile.push(discarded);
            console.log(that.gameInstance.discardPile);
            for ( let e = 0; e < theseCards.length; e++) {
                theseCards[e].removeEventListener("click", onPlayCard, false);
            }
            console.log(that.name + 'is done');
            that.gameInstance.nextPlayer( playerIndex );
        }, false);
    }
}

I tried some of the ideas from here and here, but none of them quite solved the problem.

Any helps are appreciated. I might pull my hair out soon. I thought I knew this stuff.

Community
  • 1
  • 1
McPhelpsius
  • 253
  • 1
  • 4
  • 16
  • You're adding a unique function instance to each card, but then trying to remove them all using only the function instance of the clicked card. This will only remove the one card that actually matches the instance added to the clicked card. –  Jun 10 '16 at 23:28
  • 3
    ...also, because you're adding listeners every time `takeTurn` is invoked, and not able to remove them, you end up adding multiple handlers of the same kind on each invocation, exasperating the problem. IMO, constant binding and unbinding is indication of a design flaw in your app. –  Jun 10 '16 at 23:30
  • @squint I could use some advice on how to do this otherwise. – McPhelpsius Jun 15 '16 at 03:34
  • I'll type up an example for you. Easier to do in an answer, though it will be a bit off topic, I'll touch on the reason for the initial issue too. –  Jun 15 '16 at 14:08

1 Answers1

1

The problem:

The problem is that you're adding a unique function instance to each card, but then trying to remove them all using only the function instance of the clicked card. This will only remove the one card that actually matches the instance added to the clicked card.

So element 0 gets handler 0, element 1 gets handler 1, element 2 gets handler 2, and so on...

When an element is clicked, say element 2, you then iterate all the elements to remove their handler, but you're providing the handler for the one that was clicked, element 2. The handler 2 is only useful for removing the element 2, and not the others.

The solution:

I won't actually give a solution given your current code, except to say that it would involve maintaining an array of the handlers parallel with the elements to which they were bound. It would work just fine, but I think a redesign of the app may be better.

A different approach:

IMO, repeated binding and unbinding is an indication that a different approach needs to be taken in designing your app.

You could take an object oriented approach, and represent each type of item in your app as a "class" (constructor function). So you'd have Game that manages the game as a whole, Card that represents each instance of a playing card, and Player to represent each player.

Create a Card instance for each card. Perhaps other special ones like Dealer or Deck and Discard for a discard pile. The Card could have properties representing its value and suit, and an .owner property that references the current holder of that card. The .owner could be a Player, the Discard pile or the Deck for example.

The Game has reference to all these objects, so that it can coordinate them. It deals the cards by changing the .owner from Deck to a certain Player. When a Player discards a Card, the card's .owner changes to Discard.

So basically each object maintains its own "state".

Each item could also reference its DOM element, to which you bind handlers. The handler would have a reference to the DOM element via this, but it also would need to reference the data for its associated entity. Most people would create a separate handler for each object, and use a closure to reference the data. I personally would use a little known, yet extremely useful feature called the EventListener Interface. I'll leave it to you to research this topic.

How exactly you set it up is up to you. Should the Game assign an .owner to a Card? Or the Dealer? Or should the Card be passed to an owner (like a Player) and have it assign the .owner property? Or should the Card have a method that receives its new owner and assigns it to its own .owner property? Up to you.

Hope this gives you some ideas anyway.

  • I've been using Objects like you mentioned, but your advice helped me approach my solution differently. Thanks! – McPhelpsius Jun 19 '16 at 20:57