handleOnClick() はリーダブルと言えるのでしょうか？

Question

JSのコードを読んでいるとhandleOnClick()
handleOnChange()
のようにonに対してhandleを付与するメソッド名の命名を見かけます。

メソッド名は処理内容を端的に示すべきだと思うのですが、皆さんどう思いますか？

Comments

[m.ts10806]

どれに触れるとお考えですか？
http://snowdream.github.io/javascript-style-guide/javascript-style-guide/jp/naming-conventions.html
https://www38.atwiki.jp/aias-jsstyleguide2/pages/15.html

[nyako_prog]

命名規則的には正しいのですが、リーダブルかどうかという観点での質問でした。タイトルを修正しておきます。

[m.ts10806]

となると、そのメソッドの処理内容と呼び元（要はどう呼び出されどう利用されるのか）まで必要に思います。名前だけ見てもそれが適切かどうかは判断できませんので

Answer 1

解決済のようですが、別の切り口で回答しておきます。

handleOnChange

handleOnChange() のようにonに対してhandleを付与するメソッド名の命名を見かけます。

争点はhandleOnChangeが相応しい名前かという点につきます。

• "handle" からは「イベントハンドラ」の意味を読み取れます。
• "on" からは、onclickイベント属性のように、「イベントハンドラ」の意味を読み取れます。

"handleOnChange" は重言であり、もっとスマートな名前に出来ると考えます。

適切な命名

element.onclick = onClick;
element.addEventListener('click', handleClick, false);

• 前者は "on" で始まるイベント属性名を元にイベントハンドラを暗喩するスタイル
• 後者は "handle" にハンドラの意味を込めたスタイル

MDNでは後者の命名規則を採用しており、私はより具体的な名前である後者を採用しています。

• DOM on-イベントハンドラー - 開発者ガイド | MDN

addEventListener がリスナー関数を引数にとれることから、clickListener と命名する事も出来ますが、

element.addEventListener('click', {
  handleEvent: function handleEvent () {}
}, false);

上記のように listener オブジェクトを指定した場合の関数はhandleEvent 関数となるので、両者を識別する為によりハンドラの意味が強い handleClick を採用しています。
これらは一つの考え方に過ぎず、名前が持つ意味を定義し、徹底して論理的に考察する事が重要だと思います。

疑問に思った事は、「論理的に」「矛盾がなくなるまで」考察を勧めて下さい。

onに対してhandleを付与するメソッド名の命名を見かけます。

何か思うところがある事は伝わるのですが、どのような論理で疑問を感じているのか、その点を深く追求できれば、一歩先の考えに進めると思います。

無名関数

関数型プログラミングが流行ってから無名関数が広く使われるようになりましたが、リーダブルコードの観点では無名関数は悪手だと思います。

const handleClick = event => {
  // 長い関数コード
};

document.addEventListener('click', handleClick, false);

無名関数を採用しないこのコードは本当に無意味でしょうか。
document.addEventListener のコードがすっきりしている分、可読性が上がっているという見方もあるのでは…？

const array = [1,2,3,4,5];

for (var i = 0, len = array.length; i < len; i++) {
  document.addEventListener('click', () => console.log(array[i]), false);
}

このコードはどうでしょう？
コーダの期待に反して、undefined が5回出力されますが、原因が分かりますか。

const array = [1,2,3,4,5];

function handleClick (event) {
  console.log(this.i);
}

// (A)
for (var i = 0, len = array.length; i < len; i++) {
  document.addEventListener('click', (function (i) {
    return () => console.log(i);
  }(i)), false);
}

// (B)
for (let i = 0, len = array.length; i < len; i++) {
  document.addEventListener('click', () => console.log(array[i]), false);
}

// (C)
for (let i = 0, len = array.length; i < len; i++) {
  document.addEventListener('click', {
    i: i,
    handleEvent: function handleClick () {
      console.log(this.i);
    }
  }, false);
}

// (D)
for (let i = 0, len = array.length; i < len; i++) {
  document.addEventListener('click', {i: i, handleEvent: handleClick}, false);
}

先のコードを解決する手段として、A~Dの度のコードが最も美しいと感じますか。
可読性だけでなく、機能的な違いを説明できますか。

document.addEventListner('click', function handleClick (event) {
  if (event.target.name === 'enable') {
    document.getElementById('foo').addEventListener('click', function handleClick2 (event) {
      console.log(event.type);
    }, false);
  }
}, false);

今度はどうでしょう。
このコードが持つ問題を解決するには、どのように修正すれば良いと考えますか。

---

ざっくり書きましたが、これらは初心者が無名関数を好んで使った場合に陥りやすい罠たちです。

• コールバック関数には無名関数を使うのがエレガントな書き方である

という風潮で盲目的に無名関数を使い続けると、これらの罠に陥ります。
「短く書ける」という性質に目を向ける前に、「変数のスコープ」や「スコープチェーン」にまで目を向けると、関数の位置の重要性に気が付けると思います。

あと、「無名関数式」よりも「名前付き関数式」の方がリーダブルだと思います。

Re: nyako_prog さん

Comments

[nyako_prog]

ご丁寧な回答ありがとうございました。

意味合いの被りが違和感を生んでいたわけですね。論理的で大変わかりやすかったです。

Answer 2

私は上級エスパーなので貴方の質問文の行間を読みましたが、
おそらく下記のようになっていると考えられるのでリーダブルではありません。

const handleOnClick = event => {
  document.querySelector('.foo').style.display = 'none';
  document.querySelector('.bar').style.display = 'block';
}
document
  .querySelector('.hogehoge')
  .addEventListener('click', handleOnClick); // じゃあ最初からここに無名関数で入れたら？

---

因みにhandleOnClick()は条件付きでリーダブルです。

ただし、私のイメージと一致するならばの前提です。
私がソースコード無しでその名前を見た場合、
こういう引数と処理になっているだろうと想像します。

const handleOnClick = (target, fn) =>
  target.addEventListener('click', fn);

私が法だ！！！
まぁ冗談ですけど、こういう想像をするエンジニアはまぁまぁ居ると思います。

addEventListenerが長いとはいえ
こんなラッパー関数あんまり意味がありませんよね。
関数名もこの用途ならばonClickにした方が良いと思います。

Comments

[nyako_prog]

行間を的確にお読みいただきありがとうございます。
handleOnClickは「onClickした際の挙動だからhandleつければいいだろ！」的な発想から生まれたのだと思いますが、やはりリーダブルではないですよね。

ご回答ありがとうございました！

[miyabi-sun]

ハンドラとして関数名切って管理するにしても、
何が行われるかに着目した関数名の方が良いですね。
例えばクリックするとタブが切り替わるなら`switchTab`みたいな感じでこれなら、許せる度はかなり上がりますね。