【GMOリサーチではエンジニアを募集中です】採用情報はこちら!

GMO Research システム部心得〜みんなに優しいPRの上げ方とソースの書き方編〜

GMO Research システム部心得〜みんなに優しいPRの上げ方とソースの書き方編〜

こんにちは。システム部のマツムラです!

今年の夏頃から10年前の負債を返すプロジェクトが絶賛進行中で、バタバタな毎日を送っています。

さて、私たちのチームではコードレビューに関してチーム全員がレビュワーになり開発を進めているのですが、今回のプロジェクトでもほかの人のPR(Pull Request)を見たり逆に指摘してもらうことで、バタバタな中でも日々新しい気づきがあったりします。

この気づきをシステム部みんなに共有したい・・・・!

ということで今日はタイトル通り、GMO Researchシステム部でのPRの上げ方と、ソースを書くときに注意していることについてまとめてみます!

レビューする人に優しいPRの上げ方

まずはPRの上げ方についてです。

PRは「小さい粒」であげることを気をつけています。

どういうことかというと・・

(例)自分が担当してるのは、URLを組み立てる処理

① この処理を作るには、どんな機能が必要だろう??

 ⇒ XXXtableを検索

 ⇒ ZZZZtableも検索 

 ⇒検索結果がAなら1の処理 Bなら2の処理 などなど。

② 作る順番を考える

 1.XXXtableを検索

 2.ZZZtableを検索

 3.1.2を呼び出して、何か処理する機能を作る

この場合、一番初めにXXXtableを作成するintegration層だけ作ります。

次にこのソースのTestクラスを作ると思うのですが、この時点でPRをあげてください。

すると、おおよそ2〜3個のファイルになると思います。

このようにして、小さい粒でPRを上げてあげると、レビュアーはレビューがとても楽で、

時間のかかる作業にもなりません。

ソースコードを書くときの注意点

次に、ソースコードを書くときの注意点です。

GMOリサーチ独自の変数名もバンバン使っちゃってるのですが、悪しからず・・。

function名やdataクラスの名前について

① ~data とか ~infoとかを使わない。 

→dataもinfoも特に意味をなさないので、なくてよい。

② checkStatus とかupdateEnquete とかを使わない。

→何するかわからないメソッド名はつけない

⇒isClosed~~ ,isAllowed~~,closeEnqueteなどに変更する

<参考>

クラスの命名のアンチパターン

https://qiita.com/magicant/items/8134edf969f9629fa66e

プログラミングでよく使う英単語のまとめ【随時更新】

https://qiita.com/Ted-HM/items/7dde25dcffae4cdc7923

可読性について

①早期returnできないか確認する

後続ロジックが判定結果によって不要な場合は、早期returnする。

【書き換え前】

【早期リターンに書き換えた後】

②戻り値のないメソッドについて

何かチェックして例外を投げるなど戻り値のないメソッドについては、中を読まないと何がどう処理されたのかわからないので trueやfalseを返却し、呼び元で例外処理をする。

【修正前】

このソースコードだと、呼び出し元を見た人は、service.check だけが呼ばれていて、この中でエラーが起きるのかや、これを呼ぶとどうなるのかがわかりません。

【修正後】

このソースだと、呼び出し元は、isValidTokenがNGだったときに例外を投げているので、このサービスを見た人は何が行われているかわかります。

メソッド名もわかりやすくなってます。

③ if の中にOR条件がある場合

上記のようなソースコードは、後で条件加えたりしてバグりやすいし、わかりにくいです。

(orとかandがあったら更に!)

  こういう場合は、メソッドを切り出して、このメソッド内で判定しましょう。

④ メソッドのパラメータはなるべくDataクラスを使わない

【修正前】

【修正後】

⑤ 必要なデータは必要な分だけ必要な時に取得

 

SQLを書くときは、必要な項目だけを取得しましょう。

後で使うから、ここでも使うし、あっちでも使うから・・・と色んなデータを一度でとってそのデータを引き回スト、あとで見た時にかなり分かりにくいです。

もしかして未来で使うかもしれない。

こうしておいたら未来でも使えるかもしれない。

は、実際に必要になったときに修正したり追加したりしましょう!

テストコードについて

最後に、テストコードについてです。

①Mockしすぎない

サービス内で、色々メソッドが分かれている場合はそれぞれのメソッドを個別でテストするはのはOKです。

むしろ、そのメソッドだけのテスト自体はわかりやすいですが、個別でメソッドのUTがあるからといって、Mock不要なシンプルなメソッドをMockしまくらない。

普通に動かせるのはMockしないで動かしましょう。※データ作るのが大変等でMock使う場合もあります。

②1テストケースに1つの観点だけアサートする

一個のテストケースの中に、~~が1なら、××になる。~~が2なら、▲▲になる。

というように複数の評価をしないようにします。

テストケース名が仕様を説明できなくなってしまいます。

最後に

自分の作業に追われすぎていて、他の人のPRを見る時間がなかったりすると思いますが、是非、皆さんも一度レビューをしてみてください。

レビューは、だれがしないといけないなどの決まりはありません。

勉強になるし、仕様も理解できるし、メリットばかりなので、初めてやる言語でも積極的にレビューをしてみてください。

それでは、最後まで読んでくださってありがとうございました!

社内勉強会カテゴリの最新記事