こういうケースでは早期リターンしないほうがいいかも!って話をさせてください。
例
例えば自分の Webサイト ホームページに、アクセス時の処理として以下のような実装をしたとします。
ページ読み込み後にアクセスカウンターを加算し、キリが良ければalertを出す、というものです。
const BBS_URL = 'https://..'
const accessCounter = ..
const onPageLoad = () => {
accessCounter.increment()
if (accessCounter.isKiriban) {
alert('キリ番おめでとうございます!掲示板で報告お願いします!(踏み逃げ禁止)')
if (location.href !== BBS_URL) {
location.href = BBS_URL
// ..
}
}
}
window.addEventListener('load', onPageLoad)
// ※ 例なので単純ですが、実際はもっとネストが深かったり、return後の処理が多かったりして、早期リターンしないと辛い状態だと思ってください。
これを作った数カ月後、早期リターン病を患った僕はこんな風にリファクタリングしてみることにしました。
const onPageLoad = () => {
accessCounter.increment()
if (!accessCounter.isKiriban) return // キリ番じゃなければそれ以上何もしないよ
alert('キリ番おめでとうございます!掲示板で報告お願いします!(踏み逃げ禁止)')
if (location.href === BBS_URL) return
location.href = BBS_URL
// ..
}
大満足。
―――
さらに数カ月後、僕はalert('相互リンク大募集!')
っていうアラートも表示したくなりました。
ただしこのアラートは、キリ番の処理よりも後に置きたいです。
キリ番が一番大事ですからね。
ではリファクタリング後のコードの、どの位置に新しいalertを記述しましょう?
キリ番のalertより下に配置しようと思いましたが、そこはキリ番だった人しか到達しません。
………。
…リファクタリング後のコードだと、どこに置いてもそれが実現できないことに気づいてしまいました。
ということは、早期リターンとは悪なのでしょうか? 僕の病気は治るんでしょうか?
なにが良くなかったか
今回注目するべきなのは、onPageLoad
という関数の役割です。
これは名前の通り「ページをロードしたときにやるべきことをやる」ための関数です。
つまり、アクセスカウンターを処理するためだけの関数ではありません。
元々は、偶然アクセスカウンターの処理しかやることがなかっただけです。
にも関わらず、アクセスカウンターの条件次第でこの関数全体を終了させてしまっていたことがよろしくありませんでした。
解決案1
ということは、「アクセスカウンターを処理するためだけの関数」を作ってしまえば解決しそうです。
const countupAndNotifyAccessCounter = accessCounter => {
accessCounter.increment()
if (!accessCounter.isKiriban) return
alert('キリ番おめでとうございます!掲示板で報告お願いします!(踏み逃げ禁止)')
if (location.href === BBS_URL) return
location.href = BBS_URL
// ..
}
const onPageLoad = () => {
countupAndNotifyAccessCounter(accessCounter)
alert('相互リンク大募集!')
}
window.addEventListener('load', onPageLoad)
早期リターン云々抜きにしても、タスクの切り分けができて元より良さそうです。
もしやることが
const onPageLoad = () => {
countupAndNotifyAccessCounter()
}
だけだった場合、不要にラッピングした関数のように見えるかもしれませんが、これはこのままで大丈夫です。
仕様上たまたまそのとき1つしかタスクがないだけですし、
文脈としてもそれぞれの役割に適した名前が付けられることは自然と言えます。
もちろん、新たに追加した2つ目のalertも関数にしてあげてもOKです。
解決案2
あるいは、(カウントと通知を一緒にしたくないとかで)ifが残るような場合でも、やはり関数にまとめるなどで、そもそも早期リターンしなくていいくらいに整理してもよさそうです。
const onPageLoad = () => {
accessCounter.increment()
if (accessCounter.isKiriban) notifyKiriban()
alert('相互リンク大募集!')
}
まとめ
- 「やるタスクが多岐にわたる(可能性のある)メソッド」において、あるひとつのタスクの都合次第でreturnしてしまうと後から困るケースがあるので、気をつけましょう。
- 根本的には、そもそもタスクごとに適切に関数を分けられていれば大丈夫だと思います。
という話でした。
Vue.jsのcreated
(コンポーネント作成時のフック)などがまさにやるタスクが多岐にわたる
ですが、
created () {
if (!canXxx) return
// doXxx
// ..
}
みたいなコードを見て「あっ、あっ、まだ終わらせないで!」ってことがたまにあったので書いてみました。