要素リストをループで回して addEventListener するみたいな、よくあるサンプルを断罪してみる

JavaScript 
function setButtonHandler () {
  var buttons = document.querySelectorAll(".hogeButton");
  for (var i = 0, len = buttons.length; i < len; ++i) {
    (function(n) {
      buttons[i].addEventListener("click", function() {
        alert(n);
      });
    }(i));
  }
}

って感じのコードってあるじゃないですか。JavaScript で陥りがちな落とし穴を解決するみたいな感じで。

でも、これって本当は良いコードじゃないよね。

  1. (function(n) {.... }(i)) が格好わるいっていうか、毎回即時関数実行かよっていうね
  2. click のハンドラーも同じコードなのにループ数分生成しちゃってるよねー

簡単なことを複雑にしている感じがするんですよね。(旧)JavaScript は function スコープしかなくて変数の束縛に難があるから、こうしましょうってサンプルには良いんだけど、実際にこれを使うべきかというと疑問が残るわけ。

修正案 1

function setButtonHandler () {
  var buttons = document.querySelectorAll(".hogeButton");
  for (var i = 0, len = buttons.length; i < len; ++i) {
    buttons[i].dataset.num = i;
    buttons[i].addEventListener("click", onButtonClick);
  }

  function onButtonClick () {
    alert(this.dataset.num);
  }
}

本来は各button要素のdata-num属性にあらかじめセットしておくのが正解だと思うけど、サンプルなので、ループ中に dataset.num に値を代入している。

まぁ、そこはともかく、だいぶスッキリした感じがしませんか？

修正案 2

そもそもループ自体をする必要があるのか？ まぁこれは条件が厳しいけど。一つのコンテナ的な要素内のボタンに絞れるなら、イベントの伝播を利用してハンドラを一つにしちゃうって手もあると思う。

<div id="hogeContainer">
  <button data-num="0">A</button>
  <button data-num="1">B</button>
  <button data-num="2">C</button>
  <button data-num="3">D</button>
</div>

function setButtonHandler () {
  var container = document.getElementById("hogeContainer");
  container.addEventListener("click", function (aEvent) {
    var target = aEvent.target;
    if (target.localName !== "button")
      return;

    alert(target.dataset.num);
  });
}

ループを減らすとかそもそものアルゴリズムをどうにかするのは最も良い高速化/効率化だと思うという観点から、よくあるサンプルを大人気なく断罪してみた。

まとめの様なものはない。