2
\$\begingroup\$

In my program below I am getting the parent node game 1 and then removing one of the two child nodes, depending on which one is clicked. I currently have two event listeners, with two separate functions for removing one child vs the other. It doesn't feel very dry to me, and I think I could benefit from having someone skilled in JS take a look at this and show me how this could be more easily accomplished.

var firstWinner = document.getElementById('game1');

function removefirst() {
    firstWinner.removeChild(firstWinner.childNodes[0]);
}

function removeSecond() {
    firstWinner.removeChild(firstWinner.childNodes[1]);
}

var gameOneNodes = document.getElementById('game1').childNodes;
gameOneNodes[1].addEventListener('click', removefirst, false)
gameOneNodes[0].addEventListener('click', removeSecond, false)
\$\endgroup\$
5
\$\begingroup\$

You should define one click handler that removes all siblings of the clicked element. That is, remove all children of the parent, except the target of the event itself.

document.getElementById('game1').addEventListener('click', function(event) {
    // Remove all siblings of the clicked element
    for (var c = this.childNodes.length - 1; c >= 0; c--) {
        if (this.childNodes[c] != event.target) {
            this.removeChild(this.childNodes[c]);
        }
    }
});
<div id="game1">
  <button>Alpha</button>
  <button>Bravo</button>
  <button>Charlie</button>
  <button>Delta</button>
</div>

You could also iterate this way, but there's an awkward c--:

for (var c = 0; c < this.childNodes.length; c++) {
    if (this.childNodes[c] != event.target) {
        this.removeChild(this.childNodes[c--]);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Might be useful to use this instead of parent, since this'll refer to the game1 element automatically. \$\endgroup\$ – Flambino Mar 28 '16 at 4:18
  • \$\begingroup\$ Also, while a little arcane, the for-loop can be replaced with [].slice.call(this.childNodes).forEach(...). Not that it'd be inherently better or anything, it's just an alternative. \$\endgroup\$ – Flambino Mar 28 '16 at 4:20
  • \$\begingroup\$ @Flambino I've incorporated your suggestion to use this instead of event.target.parentNode. Your slice() suggestion is too tricky for my taste, though. \$\endgroup\$ – 200_success Mar 28 '16 at 4:31
  • \$\begingroup\$ That's fair. I'm not a fan of the slice trick either, per se, but it's the simplest way to get a proper array from annoying JS/DOM array-like objects (like NodeList here, or the arguments obj). Again, not necessary in this case, but I figured I put it in a comment to mention it (no need to put it in the answer) \$\endgroup\$ – Flambino Mar 28 '16 at 4:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.