letを使わない書き方
ずっと放置してた技術ブログですが、業務でのインプット量があまりにも多く、アウトプットが追いついていないのでブログを再開することにしました。
Twitterで書いても流れるし、140文字じゃ収まらない。
しっかりまとめるほど労力もかけたくないので気軽にアウトプットしたい。
そんなレベルのものを書き連ねていきます。
自己満足感強めですが、それでもよければ見ていってください。
前置きはこれくらいにして早速内容に。
今回は業務でレビューを受けた際になるほどと思ったJavaScriptの書き方について。
結論、間違ってはいなかったけどlet使わないで書いた方がいいよというリファクタリング的なお話。
前提
- コンポーネントのstateにshowSidebarというboolean型のstateがある。
- これがtrueの時にsidebarのdivを表示させたい
疑似コードで書くとこんな感じ
renderSidebar = () => { //もしshowSidebarがfalseならば sideBar = null //もしshowSidebarがtrueならば sideBar = ( <div> //sideBarの内容 </div> ) } ... return ( <div> //maincontentsの内容 {this.renderSidebar()} //sideBarの呼び出し ... </div> )
sideBar部分はrenderSidebarという関数で呼び出しておいて、showSidebarがtrueの時だけsideBarに内容が入るためそれが表示されるという仕組み
実際の実装
これを最初次のように実装していた。
renderSidebar = () => { const { showSidebar } = this.state; let sideBar = null; if (showSidebar) { sideBar = ( <div> //sideBarの内容 </div> ); }; return sideBar; }; ... return ( <div> //ここにmaincontentsの内容 {this.renderSidebar()} //sideBarの呼び出し ... </div> )
return内でrenderSidebar関数を呼び出し、showSidebarがtrueかfalseかで表示させる処理内容を変えるというもの。
ここでletを使った変数sideBarにnullを代入しておいて、showSidebarがtrueならdivを再代入してreturnしている。
ここをletを使わないで実装できるならその方がいいとのこと。
理由はこの2つ。
- コードが見やすくなって簡潔になる
- 予期せぬところで再代入が起こり、バグの原因になるのを避ける
2番目の理由が大きそう。
このレベルの処理なら大丈夫だけど、使わないにこしたことはないということなのだろう。
実際調べてみるとletはfor文の最初の宣言だけにすべきで、基本全部constにすべき的なのをよく見る。
後半は問題ないので、前半の関数の重要な部分だけ抜き出して書いた改良版がこちら
if (showSidebar) { return ( <div> //sideBarの内容 </div> ); }; return null;
そもそも変数すら使わずにtrueの場合にsideBarのdivをreturnする。
関数内でreturnされるのでその後の処理は行われない。
これによってtrueの場合は最初のif文が実行されてdivがreturnされる。
falseの場合はif文が実行されず文末のnullがreturnされる。
という処理になる。
確かに見やすくなった。
ここから先のリファクタリングはもはや宗教になりそうというお言葉と共に、さらなる改良版を教えていただいた。
nullを先に返した方が可読性が上がると思うとのこと。
それがこちら。
if (!showSidebar) null; return ( <div> //sideBarの内容 </div> );
うん、個人的にはこれが一番好きだ。
かなり見やすいし、スッキリしている。
showSidebarがfalseの時はnullを返してその後の処理は行わない。
先ほどのものと処理が逆になっただけだが、if文のネストがなくなっただけでもかなり見やすくなっていると思う。
Reactはコード量も多くなりがちなので、小さなことでもコードの可読性はこれからも意識していきたい。