イベントハンドラの関数名について初心者エンジニアが考えてみた #React

はじめに

Xでイベントハンドラの関数名についての話題をみて、その見解についてのこちらの記事を読ませていただきました。

ちょうど今週実装した内容で関数の命名とイベントハンドラの書き方について先輩に質問した内容があったので振り返りつつ、記事の内容を踏まえて自分の書いたコードをリファクタしてみます。

どんな命名が適切なのかは私の経験が浅いため今は分かりませんが、色々な考え方を吸収して、考えていきたいと思います🙌

自己紹介

私は2023年9月から、React / React Native / TypeScriptを扱うフロントエンドエンジニアとして自社開発のSaaS企業に入社しました。未経験独学で、薬剤師からの転職です💊
日々の業務で学んだことや、難しかった内容についてQiitaにアウトプットしています。

実装内容

背景

新規プロダクトのごみ箱機能を実装しています。
既存プロダクトのソースコードを参考にしながら、実装を進めていました。2つのプロダクトで、書き方に違いがあったのでその意図やどちらが好ましいのかを質問しました。

要件

ごみ箱のページに選択したアイテムを削除するボタンと、復元するボタンを画面に追加する
削除ボタンをクリックしたら削除確認のコンファームを表示する
復元するボタンをクリックしたら復元処理を実行する
アイテムを何も選択していないときは、ボタンは非活性

私の実装

以下が私が最初に書いていたコードです。
関数名はonClick〇〇Buttonです。削除/復元ボタンを押した時に何かしらの処理が行われますが、子コンポーネントはその内容については関与しません。

container.tsx

// 親コンポーネント
const checkedItemCount = checkedTrashIds.length

const onClickDiscardButton = (): void => {
    if (checkedItemCount === 0) {
        return
      }
    // ...削除確認のコンファームを表示する
}

const onClickRestoreButton = (): void => {
    if (checkedItemCount === 0) {
      return
    }
    // ...復元実行
  }

return (
    <Children
      checkedItemCount={checkedItemCount}
      onClickDiscardButton={onClickDiscardButton}
      onClickRestoreButton={onClickRestoreButton}
    />
)

presenter.tsx

// 子コンポーネント
<button onClick={onClickDiscardButton} disabled={checkedItemCount === 0}>
    削除
</button>

<button onClick={onClickRestoreButton} disabled={checkedItemCount === 0}>
    復元
</button>

既存プロダクトの実装

一方で、既存プロダクトのソースコードでは以下のように書かれていました。
注目すべきは、onClick属性への関数の渡し方と関数名が異なることです。

Container.tsx

const onDiscardTrash = (): void => {
    // ...削除確認のコンファームを表示する
}

const restoreTrash = (): void => {
    // ...復元実行
  }

return (
    <Children
      checkedItemCount={checkedItemCount}
      onDiscardTrash={onDiscardTrash}
      restoreTrash={restoreTrash}
    />
)

Presenter.tsx

<button
    onClick={checkedItemCount === 0 ? undefind : onDiscardTrash}
    disabled={checkedItemCount === 0}
>
    削除
</button>

<button
    onClick={checkedItemCount === 0 ? undefind : restoreTrash}
    disabled={checkedItemCount === 0}
>
    復元
</button>

onClick属性への関数の渡し方

私は関数の中に、「アイテムを選択していないときは何もしない」という処理を書いていました。
一方こちらは、関数の中には書かずにonClick属性に渡す式の中に条件式を書いています。

後者の場合、子コンポーネントファイルを見ただけで「アイテムを選択していないときはクリックしても何も起こらない」ということがすぐにわかる点が良いと思いました💡✨
一方で、イベントハンドラにはクリックしたら何が起こるか？という処理を書くものだと思っていたので、UIを担当するファイルの中で表示とは異なる処理を書いていることに違和感を感じました🤔

関数名

「私はボタンを押したとき」という意味で、復元・削除ボタンのどちらの関数にもonClick〇〇Buttonと命名していました。
一方こちらは、クリックした時の処理内容によって関数名が異なっています。

削除ボタン
削除ボタンをクリックしたときは削除確認のコンファームを表示します。クリックしたときに削除が実行されるわけではありません。onDiscardTrashという命名にすることで、ボタンをクリックした時に何かの処理が実行されることが読み手に伝わりやすいと思いました。
復元ボタン
復元ボタンをクリックしたときは復元処理が実行されます。restoreTrashと命名することで、ボタンをクリックした時に復元処理が実行されることが読み手に伝わります。また、onRestoreTrashとは命名しないことで、削除ボタンとは処理が異なるのかも？と推測できると思いました。

どちらが適切か？

先輩に確認したところ、結論どっちでもいいと言われました。
ただ、後者のようにonClick属性に登録するイベントハンドラの中で条件式を書くなら、checkedItemCount !== 0のときに何をするか？ということがわかる関数名にした方がいいので、onClick〇〇は不適切だと教えていただきました。

リファクタ

では記事の内容と既存プロダクトの書き方を踏まえて、自分で書いたソースコードをリファクタしてみます。

onClickやonSubmitといったイベント名が直接現れるpropsは、DOMやUIを直接的な責務とする、末端のコンポーネント（あるいはネイティブのHTML要素）にのみ現れるものなのです。

がっつり親のContainerコンポーネントにonClickというイベント名を書いてしまっているので直したほうが良さそうですね。

// Container.tsx
const showDiscardConfirm = (): void => {
    if (checkedItemCount === 0) {
        return
      }
    // ...削除確認のコンファームを表示する
}

const restoreTrash = (): void => {
    if (checkedItemCount === 0) {
      return
    }
    // ...復元実行
  }

return (
    <Children
      checkedItemCount={checkedItemCount}
      showDiscardConfirm={showDiscardConfirm}
      onRestoreTrash={restoreTrash}
    />
)

Presenter.tsx

<button onClick={showDiscardConfirm} disabled={checkedItemCount === 0}>
    削除
</button>

<button onClick={onRestoreTrash} disabled={checkedItemCount === 0}>
    復元
</button>

修正点

削除ボタンをクリックした時のイベントハンドラ関数名
onClickDiscardButton=>showDiscardConfirmに変更しました。
クリックしたらコンファームが表示されることが読み手に伝わりやすくなったと思います。
復元ボタンをクリックした時の関数名
onClickRestoreButton=>restoreTrashに変更しました。親のContainerコンポーネント側は、この関数がいつどのように使われるかは知らず、復元処理のみ責任を持ちます。
復元ボタンをクリックした時のイベントハンドラ関数名
親コンポーネントからpropsをonRestoreTrashという命名で渡すように変更しました。これでbutton要素が書かれている子コンポーネントだけ、復元ボタンをクリックした時に復元処理が実行されることが伝わるのではないでしょうか。

終わりに

リファクタしたコードを見てみたのですが、本当にこれでいいのか？と考えてしまいました。

削除ボタンを押した時なんだから、onDiscardTrashの方がいい？
いやでも、ボタンを押した時に削除されるわけではないし…
復元ボタンを押した時に復元されるんだから、素直にrestoreTrashの方がいいのでは？？
ってか既存プロダクトみたいに子コンポーネント側で何も選んでないならクリックしても何も起こらないことがわかったほうがいい？？？？
それともhandle〇〇でどちらも書いた方が揃っていて綺麗？？？？？？？？？

いや命名難しすぎるって！！！！😂😂😂

正解がないからこそ、色々な考えを持ったエンジニアの方がたくさんいるんですね。
プロジェクトの中で伝わりにくい書き方をしてしまうと、チーム内で混乱を招きそうなので、どんな命名をしたらいいか先輩にも確認する必要がありそうです。
エンジニアになって約5ヶ月経ちましたが、最近関数名だけでなく、変数名も難しいと感じていたので、記事なども参考にして、根拠を持って命名できるようになりたいと思います。