実務未経験でプログラマとして入社して半年以上が経った。
コードレビューで指摘されたことを備忘録としてまとめておく。
自分なりにまとめたものなので、レビュアーが言いたかったこととニュアンスや解釈がずれている可能性はある。
初歩的な内容ばかりで我ながらうんざりする。
せっかく優秀な同僚ばかりなのだからもっと高度なことを学びたいが、こういう初歩的なことが出来ないのが俺の現状なのだから、仕方ない。
そもそもPullRequestを送ったこともなかったわけだし。入社初日は、一人でPullRequestの出し方を練習していた。
それを考えればまあ、こんなものだろうか。
当たり前のことをちゃんと当たり前に出来るようになって、早く、次のステージに進みたい。
PullRequest(PR)
- PRのタイトルは分かりやすいものに。必要に応じてチケットの番号なども入れる。
- コミットやPRは出来るだけ粒度を細かくする
- 余計な差分が含まれないように注意する
- 例えば、ライブラリのバージョンなど
- 余計な差分が含まれないように注意する
- まとめるべきコミットは、一つにまとめる
- 変更点が大きなる場合は、featureブランチを作って、そこにPRを出していく
- bundleされたファイルなどがちゃんと圧縮されているか確認する
命名
- 変数名は保守性に大きく影響する
- 関数名は意味を絞る。例:
isAvailable
→isOrderable
- 関数名は何が返ってくるかイメージしやすい名前をつける。
- データ型を変数名に含めるのではなく、可能な限り英単語として示した方がコードが読みやすい
- 適切な名前を出すことが出来ない場合、設計や抽象化が間違っていることが多い
設計
- 関数やコンポーネント毎の役割を正しく分ける。異なる役割を担わせたり、責務が曖昧になることのないよう、正しく分離する。
- 可読性の低い処理は関数に切り分けて、処理の流れの見通しをよくする。
- ある関数をラップして使うことで、関数の処理を共通化できる
- 関数に渡す値、関数が受け取るデータの構造、というのはよく考える
- 関数ではなく呼び出す側で、処理を行う。関数の役割や責務を明確にする
- 本当にそのデータ構造の引数が必要なのか、考える。もっとふさわしいものがないか。
window.xxx
などは関数のなかで処理するのではなく、引数として渡したほうが、関数がテスタブルになる。- 必要なデータのみを、関数やクラスに渡すようにする
- プロパティは出来るだけシンプルに。ただ真偽値を持たせるだけのプロパティで、使用ケースが限られているのなら、専用の関数を作って呼び出す形にするのがいいかも。
isXXX
のような。 - データ構造を考えるときは、RDBの設計をイメージする
- 同じデータを複数作ってはいけない。不整合が起きる
- こういうときも、データベースをイメージする。あるテーブルの情報を主キー以外のキーでも参照したい時に、そのキーで同じデータをinsertしたりはしない。それと同じ。
- データ構造はよく考える。正しさや適切さを考える。主キーは何か、など。
- 変更内容によっては、後方互換性も意識しながら書く。URLの変更、ブラウザに保存するウェブストレージの構造の変更、など。
- URLの階層構造は、よく考える。拡張性や、開発のしやすさ、階層の意味が正しいか、など
- URL、階層を増やしたりすることを恐れない。無理に既存の仕様に押し込まない。
- 外部サービスの仕様を前提にして設計するのは、よくない。仕様が変わるかもしれないし、そのサービスを使わなくなるかもしれない
- 機能を拡張する場合などは、TDDのような形で先にテストを書くとよい
コーディング
- 起こりえない条件に対してコードを書くのは、それが起こりえることなのかと混乱させてしまうので避けた方がいい
- マジックナンバーは避ける。
- ドメインロジックがビューに露出するのは好ましくない
- 非同期処理の
catch
は、エラー処理のために使うべきで、エラーを回避したり握り潰したりするのに使うべきではない。 - 処理の流れを分かりやすくする。頭のなかの変数を少なくする。
- 例えば、ガード節などを使う
- 余計な再代入などを行っていないか気を付ける
- 三項演算子や短絡評価を使って、コードを簡略化する
- 条件分岐の条件は、直感的に分かりやすいようにしよう
- 条件分岐において、否定の複数条件はわかりにくい
React
- なんでもかんでも
state
にするのではなく、必要ならクラスのプロパティとして持たせることも検討するstate
とプロパティの使い分け。基本的には、値が不変のものはプロパティ、変化するならstate
にする。
- JSXの
spread attributes
を使って、表記を簡略化する - Reactでアプリ作る場合、それぞれのComponentが担当する責務やレイヤーを意識してコード書くとよい
- セマンティックなマークアップを心掛ける。安易に
<div>
と<span>
を多用しない。 - ちょっとしたDOMを挿入するくらいなら、わざわざReactを使う必要はない
CSS
- リンクカラーなどは、ページ全体で定義するのが大前提
- そのうえで、必要に応じて個別に設定していく
- そういったことをし易い設計にしておくべき
- 画像やCSSがどこのパスから配信されているか確認し、ここにも適宜手を加える
- 必要なら共通化も行う
- CSSで設定した値を初期値に戻す方法は、
initial
やauto
があるinitial
はIE11が未対応なので注意- https://caniuse.com/#feat=css-initial-value
- クラスの名前は、実装ではなく役割に合わせて行う。後に実装が変わっても名前に影響が出ないように
JavaScript
- JavaScriptの変数名はローワーキャメルケースで統一するのが基本。APIから取ってきた情報がスネークケースであったとしても。
Array.prototype.find
は、IEでは動かない- フロントエンドをやる以上、ブラウザ対応、というものは必ず意識する
Object.assign()
の仕様を理解し、基本的には第一引数には空のオブジェクトを渡すようにする- 最大値を返すならMath.maxを使って実装した方がわかりやすい
テスト
- 仕様が変わった際は、それに合わせてテストで使うデータ構造なども変えていく
- テストのためだけにpublicにするのは避ける。外部からアクセスできるようにするとメンテナンスが大変になるから。
- 手動テストのことも意識する
- そのためにも、「実装方針」だけでなく「仕様」も固めていく
プロジェクト管理
- 関連するAPIなどの状況も確認しながら作業する。リリースできるのかどうか。
- 変更内容によってはNginxなどのインフラの設定も必要なので、それを忘れないようにする
- 未知の部分を早めに出し尽くして、不透明な部分を消していく
- 方針がある程度まとまって作業に入るなら、作業内容や工数を整理して、共有しましょう
その他
- Google Analyticsなどを利用している場合、データが正しく送信されているか確認する
- バグの対応は、以下のような手順で
- バグを再現させるテストコードを書く
- バグを修正する
- バグが再現しないことを確認する
- SPAのエントリ部分に手を加えると、Nginxなどのインフラ側の作業が発生する可能性がある