リファクタリングしてみた

今回は輪読会の課題図書であった、リーダブルコードを読んで、リファクタリングしたお話を書いていこうと思います。ソースコードをレビューして怒られると言う経験はエンジニアを強くするんやで！でもへこむから優しくしてください。気遣いのできるエンジニアになりたいと思う今日この頃です。

どもども！最近めちゃめちゃ熱くないですか？溶けそうなのでクーラーに全力で仕事をしてもらって悠々と仕事をしている龍ちゃんです。弊社の同期で「輪読会」なるものを開催しています。ちょうど一か月かけて、ブログの資料としてまとめる作業をやっていたのですが、これはブログの投稿数も上がる良い取り組みだなっと思っています。ちなみに発案は僕ではありません。僕は輪読会の話が上がった横で、「上司の金言をブログにまとめる話」をしていましたｗ。以下が輪読会系統で書いたブログ記事になります。

いや～これだけ並ぶと圧巻ですね。この取り組みだけで、一か月に参加人数分だけブログの投稿数が加算されるので、ブログをたくさん書いている身からするとうまうまですね。

さて、今回はそんなリーダブルコードを読んでリファクタリングしてみたよ！というお話です。実際に僕がレビューで言われた内容については「コードレビューは学びが多い」にて書いています。まぁここを読んでわかることは、僕のコードは悲惨であるという話ですね。せっかく「リーダブルコード」も読んだことですので、プロジェクトでリファクタリングに取り組んでみました。

では本編をぬるっと始めていきましょう。

本題というかなんというか区切りをつけたいためのタイトル。

さて、ぬるっと本編が始まりました。とりあえずリーダブルコードで身に着けた「言語化」というスキルを駆使していきたいと思います。今回やりたいことの説明ですね。

解決したい問題は「直リンクのルーティング制御」になります。フロントエンドエンジニアなので、React:SPAだとよく組む構成ですね。この条件を言語化すると以下になります。

ルートは/subscription/upgrade/****といった具合にルーティングされる。
/subscription/upgrade/confimationと/subscription/upgrade/completedは直リンク禁止で/subscriptionにリダイレクト
StandardPlanを契約している場合は/subscriptionにリダイレクト
/subscription/upgradeと/subscription/upgrade/payment-inputは直リンクOK

これを頭の中で構築して作り上げたコードが以下になります。

useEffect(() => {
    const pathName = location.pathname;
    if (upgradeFirstEntry != undefined) {
      if (standardPlan && !isLoading) navigate("/subscription");

      if (pathName == "/subscription/upgrade" || pathName.includes("payment-input")) {
        setUpgradeFirstEntry(false);
      } else {
        if (pathName.includes("confirmation") || pathName.includes("completed")) {
          if (upgradeFirstEntry) navigate("/subscription");
        }
      }
    }
  }, [location]);

はいはい、んでこれをリファクタリングするとこうなりました。

useEffect(() => {
    const pathName = location.pathname;
    const allowEntry = pathName == "/subscription/upgrade" || pathName.includes("payment-input");
    const needValidate = pathName.includes("confirmation") || pathName.includes("completed");

    if (upgradeFirstEntry != undefined) {
      if (standardPlan && !isLoading) navigate("/subscription");
			if (needValidate && upgradeFirstEntry) navigate("/subscription");
      if (allowEntry) setUpgradeFirstEntry(false);
    }
  }, [location]);

もうぱっと見で見やすさが伝わればうれしいです。さてここでブログを書くのをやめてしまうのはもったいないので、学んだことで活かせたなと思う点をまとめていきたいと思います。

立ち話：リファクタリング

まぁ立ち話みたいなものなので軽い口調で書いていきますね。この行の後はバリカタで書いていますけどねｗ。今回学んだことで、生かしているのは以下の3点です。

言語化：やっていることをNotionに書き出して整理（時間ない時は口頭で説明）
ガード節：ifでネストするよりも処理で切り出してしまえばよくない？
命名：それだけ読んだらわかる名前で書く（英単語を調べて書けばよい）

「言語化」は先ほどちょろっとやっていましたね。やりたいことを整理すると言う表現が一番わかりやすいと思います。エンジニアは脳内でこのフェーズをやりがちです。スーパーエンジニアの場合は、それで問題ないかもしれません。僕のようなジュニアエンジニアは詰まった際は、冷静に立ち止まって整理することが重要だと学びましたね。まぁ立ち止まりにくいからジュニアなんですけどねw

「ガード節」は記法の話になります。if文でネストをしていくコードよりも読みやすさが上がると思います。これは輪読会で、いつもコードレビューをもらっている上司から教えてもらいました(最近コードを書く時は心の中に小さな上司を召喚してコードを書いています)。if分のネストを軽くする方法として、ガード節を用いています。Qiitaでよさげな記事があったので共有しておきます。今回は、条件に一致していない場合リダイレクトという流れになっているため、ガード節を使用しています。ネットでガード節について検索をすると、「ガード節禁止」や「アンチ・アーリーリターン」といった内容も出てくるので、用法容量を守って使用しないといけないということがわかります。結論としては、見やすいコードは正義ということなので、見やすいコードになればそれでよいと思います。積極的にレビューをして・されていきましょう。

「命名」は読んで字のごとくですね。わかりやすい名前を付けましょうというお話です。リーダブルコードを読む前に書いたコードでは、「temp」や「list」といった雑な宣言がいっぱいあります。まぁリーダブルコードでも重要じゃないコードはそういった書き方をしても問題はないという話ですが、条件に使用している変数まで雑な名前を付けていたので、上司に注意されました。重要な変数に関してはええ感じの名前を付けてあげましょう。悩んだときはネットに聞くのもよいでしょう。こちらのサイトはちょうどコードを書いているときに参考にしたサイトです。現代ですと、ChatGPTに聞くのもありですね。

まとめ

さてさて、今回はちょっとしたまじめ内容をフランクな感じに書いてみました。リファクタリングって難しいですよね。でもよいコードをかけるエンジニアは強つよエンジニアだと思うので、ジュニアからシニアに年齢だけでなくレベルも上がるようにしていきたいものです。

ちょうど昨日ですが、エンジニア四人で一つのコードを「あーでもないこーでもない」といいながらリファクタリングしてみました。激ムズでしたが、集まってするリファクタリングはちょっと楽しかったです。というか人のコードを読むのは難しいですね。実際リファクタリング用のコードはややこしい感じで書いてあるわけですが、僕も似たようなコードをレビュー依頼出したことがあるので反省です。心の中の上司とリアル上司に謝罪をしながら、今日の朝はすっと隣の席を占領してました。

今日は金曜日なので、来週も頑張れるようにリフレッシュをしてきます。ではまた～

RyosukeTanaka

LOG

本題というかなんというか区切りをつけたいためのタイトル。

立ち話：リファクタリング

まとめ