はじめに
どもども!こんにちは、人生初めてのコードレビューをいただいてテンション爆上がりな龍ちゃんです。コードを書き始めて10年ほどたちます。ある程度の金額をいただいて働き出して「会社員としてのコード」ということで、レビューをいただいて興奮しましたね。内容を見てダメージを受けたんですけども…
そのままの勢いで、まとめていこうと思います。レビューをもらえたことがうれしすぎて、すべてのレビューに対してコメントを出しちゃいました。レビューをお願いしている方が忙しいこともあってなかなかコメントをもらえないので、貰ったコメントを噛みしめてよいプログラマーを目指いしていこうと思います。
それでは、サクッとまとめていこうと思います。最近は時間術の話をしたりしているので、何か時間術の本でも読みましょうか。
コードレビューから学んだこと
それでは本題に入っていきましょう。今回レビューで受けたことは10個ほどあったのですが、実際のコードからの指摘が多く存在しました。大きく分けると以下の3つに分類できるような印象でした。
- コーディングテクニック
- 本番環境と開発環境の切り分け
- 複数人で開発するためのTips
これだけ書いてもよくわからないと思うので、実際言われたことをの抽象度を上げてまとめたのが以下の図になります。
抽象度を上げすぎてわからなくなっている部分もあるかと思うので、節に分けて触れていこうと思います。このブログを読み切った後にはわかるように懇切丁寧に書いていきますね。「本番環境にconsole~」や「使っていないファイルは削除しよう」の2つはそのままですね。今回の場合だと、最終的にお客さん先にデプロイをすることが目標となります。デプロイする際にデバック用の「データがきた~~」というコメントが出たらまずいですよね(実際に開発中に書いてましたww)。あとは、検証用のファイルが混入するのもレビューの際に、見間違いなどが発生するパターンもあるので使用していないファイルや不要なコンソールは極力消す対応をしましょう。
コーディングテクニック
ここでは、先ほどの図で言うところの1・2・4と関連があります。主にレビューのやりやすさや新しくコードを読む方のためにという側面が強いと思います。関連している内容としては「ハードコーディング」・「命名規則」・「コメント」の3つです。
ハードコーティング
ハードコーディングというのが何かわからない方はこちらにリンクを置いておきますね。ここは感想になるのですが、「ハードコーディングはよくないな~」というものです。今回は実装の都合上リダイレクトURLなどを指定していたのですが、そこが間違っていて動かない問題などもありましたね。極力ハードコーディングはしないようにしていきましょう。そして、これは次節の「本番環境と開発環境の切り分け」にもつながってくるのですが、本番環境と開発環境でリダイレクトURLを変更しなければならないので大変になりますね。
今回の対応策としては、ハードコーディングしていた部分を動的に変更するためにURLを動的に取得しました。JavaScriptのlocation.hrefを使用しています。URLの切り分けに関しても説明があるので知らなかった人は覚えましょう。
const locationURL = location.host;
console.log(locationURL);
命名規則
命名規則は難しいですね。これは指摘が多かったのでこれから直していきます。とりあえず以下の図を見てください。これは今回のレビューで実際に修正した部分になります。
機能としては、「認証が通っていればUserの情報を取得する」というものになります。レビューで言われた内容としては、「isAuthCheckだと何を取得しているのかわからず、中のコードを読む必要がある。またCheckという単語は頻出単語なのでなるべく使用を避けたほうが良いですよね。」ということでした。確かに、「isAuthCheck」だと認証状態を監視しているように感じますね。なので命名を「authUserData」に変更しました。また、認証状態を監視する「isAuthenticated」という変数を用意して機能を分割しました。
僕が良く使用している一時避難的な変数が下になります。
- temp
- count
- value
これを他人に見せた場合だと、何を表しているかコードを見ないといけないですよね。なるべくこういう定義を削除してわかりやすい命名にしていきたいと思います。
命名は普段から気にしていかないと矯正が難しいので、今日からわかりやすい命名規則を始めていきます。これを読んでいる皆さんも納得したら今日から気を付けましょう。
コメント
あなたはコーディング中に日本語を書く機会がありますか?僕は正直書くことが得意ではありません。先ほどの「命名規則」に関連している部分ですが、ロジックが複雑化した際にコメントが残っていることで助かる場面は多くあります。命名によるわかりやすさもありますが、コメントを日本語で読むほうが楽な場合は存在します。
コメントの書きすぎはよくないのですが、解説を入れたい場合やロジックを端的に表現する際には日本語で簡単に書いた方が楽ですね。コメントのメリットは「複数人で開発やレビューするためのTips」にも関わってきますので、気になる場合はそこに飛んでください。
本番環境と開発環境の切り分け
ここでは、先ほどの言うところの1・3・5と関連があります。ここは、商用コードを書く方にとっては基本になる部分になるかと思います。正直、独学でプログラムを書いている際には絶対たどり着かなかったリファレンスを読むことになったりするので、就職した利点が生きているかなと思います。
本番環境と開発環境がおんなじ環境ということは、基本的にはありません。変わる部分としてWebサービスで分かりやすいのはURLや認証の接続先です。URLは「http://localhost」と「https://xxxxxxxxxxxxxxx」といった具合に変わるのでとても分かりやすいです。この部分は「ハードコーディング」と関連している部分になります。リダイレクトURLはデプロイ先が変わる場合に、文字列を書き換えないといけないので取得するように変更しました。
本番環境と開発環境を分割するテクニックとしては環境変数や「.envファイル」といった方法が考えられます。これはプロダクトで使用している言語や環境によって手法が変わるので、その都度選定という形になるかと思います。必要な場合は有識者に相談しましょう!
明確に本番環境と開発環境で変えなくてはいけない部分が3・5の部分になります。今回初めて知った単語としては「ソースマップ」という単語があります。このソースマップやconsoleが本番環境に残っていると「素人感」というのが残ってしまいます。商用コードとしては、こういった素人感が残るものを削除していこうという話ですね。同様の理由で使用していないファイルは削除しておきましょう。ここの理由としては、ビルド速度の向上などの理由があったりするのですが、使ってないファイルもリポジトリには残っていますからややこしいので消すのが無難です。
これも新出の技術だったのですが、「Terser」という技術でビルドする際にconsoleを消し飛ばす技術らしいです。知らなかったのですが、泣きながら導入しました…
複数人で開発するためのTips
ここでは、先ほどの言うところの4と「コーディングテクニック」に関連があります。これはチーム開発をする際に影響が大きいトピックとなっています。複数人で開発経験が入社してからの僕としては、この部分も新しい知見ということでこれから気を付けていきたい部分でもあり、就職した利点でもありますね。
レビューをもらったプロダクトでは、デザインツールとして「Figma」・技術としては「React/Typescript /Tailwind」を使用しています。このFigmaとの連携が取れていない部分に関してコメントをいただきました。Figmaで設定した名前とコンポーネント名がずれていたため、レビューのしやすさを上げることができます。コメントでも残しておくと助かりますとコメントをいただいたので、コンポーネント名から変更しちゃいました。
仕様書を作りこむことも複数人で開発する際に必要となる部分でもあります。仕様書で定義したとおりに開発を進めていくことで、仕様書からレビューにスムーズに導入することができます(仕様書の作り方は難しい…)。
おそらくもっと多くのTipsがあると思います。今回のコードレビューから得たTipsとしては以下の2点です。
- デザインツールとの連携を取りやすい命名規則やコメントを付ける
- 仕様書は協議を取ってしっかり作りこむ
これから見つけたらブログに書いていきますね。
終わりに
はいはい!お疲れ様です。今回はとてもスムーズに書き上げることができました。よほどコードレビューがうれしかったんでしょうね。コードレビューは結構な時間を取らせてしまうので、学べることはすいだしていきましょう。レビューがもらえる環境は超貴重です!!上司のSさんにはここから愛を叫んでおこうと思います。
さてさて、次のプロジェクトではもっといいコードを書けるように勉強していこうと思います。実はコード規範といったものはまったく学んでこなかったので、これから勉強していくしかないですね。
この話を同期としていたら一冊の紹介をされましたので、「リーダブルコード」のリンクを置いておきます。読んだことないので、ちょっと買ってきます。世の中のエンジニアの皆さんが要約をまとめてくれているので、リンクだけ置いておきます。これだけリンクが出てくるということは良い本なのでしょう。