0

I'm creating a game played by two people where an active player changes on a button click. The 1st code only changed the activeplayer once in two clicks. When I rewrote the code into the 2nd one, the problem got solved.

1st

document.querySelector('.switchPlayer').addEventListener('click', function(){

   //switch current player
   currentPlayer === 1 ? currentPlayer = 2 : currentPlayer = 1;

};

2nd

document.querySelector('.switchPlayer').onclick = function(){

    //switch current player
    currentPlayer === 1 ? currentPlayer = 2 : currentPlayer = 1;
};

After googling, the only thing I could understand was that addEventListener can have multiple handlers as opposed to onclick which can have only one handler.

If anybody could teach me why the 2nd code works, I would appreciate it.

Tadashi
  • 9
  • 2
  • Does this answer your question? [addEventListener vs onclick](https://stackoverflow.com/questions/6348494/addeventlistener-vs-onclick) – Tanner Dec 31 '19 at 14:31
  • 2
    It's supposed to be `currentPlayer = currentPlayer === 1 ? 2 : 1;` or simply `currentPlayer = 3 - currentPlayer;` (also, original code seems to work fine: https://jsfiddle.net/khrismuc/c7964Lwx/) –  Dec 31 '19 at 14:32
  • Do you only ever run this code once? Or do you run it repeatedly? – T.J. Crowder Dec 31 '19 at 14:32
  • 1
    Side note: I'd strongly recommend that you don't use the conditional operator as a poor man's `if`, where both the true and false conditions are executed purely for their side effects. Either use `if`, or when (as in this case) you're assigning to a single variable in both cases, just use the conditional on the right-hand side of the assignment: `currentPlayer = currentPlayer === 1 ? 2 : 1;` – T.J. Crowder Dec 31 '19 at 14:34
  • 1
    The main difference is that calling `addEventListener`, as the name suggests, keeps adding new event listeners. If you ran the first snippet twice for instance, both listeners get called and the variable is toggled two times. Assigning to `onclick` however overwrites the previous handler function, so having it two times will "work". My guess is you accidentally put that code in a place where it gets called repeatedly. –  Dec 31 '19 at 14:36
  • *"The 1st code only changed the activeplayer once in two clicks."* [Works for me](https://jsfiddle.net/tjcrowder/9f3qycmd/), if I add the missing `)` and assume a `currentPlayer` variable is in the containing scope. – T.J. Crowder Dec 31 '19 at 14:36

1 Answers1

-1

The best way to attach the listener is to use addEventListener. This way gives you more control and also provided the event object as the first parameter in the event handler.

The documentation on MDN states the following:

Only one onclick handler can be assigned to an object at a time. You may prefer to use the EventTarget.addEventListener() method instead, since it's more flexible.

Also, I would use 0 and 1 to represent player 1 and 2 respectively, because this reduces the logic for swapping to a simple arithmetic-based switch.

var currentPlayer = 0; // Current player (global)

addEvent('.switchPlayer', 'click', handleSwitchPlayer);

/*
 * Switches the current player by toggling between 0 and 1.
 */
function handleSwitchPlayer(e) {
  currentPlayer = 1 - currentPlayer;
  console.log('Swapped to player #' + (currentPlayer + 1));
}

/**
 * Adds an event litener to element(s) by selector.
 */
function addEvent(elementOrSelector, eventName, eventHandler) {
  if (typeof elementOrSelector === 'string') {
    elementOrSelector = document.querySelectorAll(elementOrSelector);
    Array.from(elementOrSelector).forEach(element => {
      element.addEventListener(eventName, eventHandler);
    });
  }
}
<button class="switchPlayer">Switch player</button>
<button class="switchPlayer">Also switches player</button>
Mr. Polywhirl
  • 42,981
  • 12
  • 84
  • 132