初めまして、新卒エンジニアの@reiya018です。
この記事では今まで新卒入社後の配属から私が受けてきたGithubのPullRequestのレビューをいくつか抜粋して振り返ろうと思います。 4月~5月は会社の研修のため開発をしていませんでしたので、6月~11月の約半年の間でいくつかピックアップしてみました。
対象のリポジトリは弊社が運営するマッチングアプリのAPIサーバーとして利用しているバックエンドのRailsのコードです。
この記事の目的
この記事を書く目的は3つあります
・ 自分の振り返りのため
これが一番大きいです、一度配属から今までを振り返って何がダメで何を改善できるかを整理したいと思ったからです。
・ 同じ新人エンジニアの方に見てもらいたい
他の新人がどんなミスをしているのか知って反面教師にしてもらいたいです。
・ 指導するエンジニアに初心者の気持ちを知ってもらいたい
新人エンジニアがどのようなミスをしているのか・やりがちなミスは何か、何がわからないのかを知ってもらい、教育の助けになればいいなと思います。
PullRequestについて
私の部署ではPullRequestは必ず二人以上のApproveと外部のテスターさんのテスト(小規模であれば自分たちでテストする)しないとマージできないようになっています。そのため先輩エンジニアにレビューをお願いするとたくさんコメントが返ってきます。
先輩エンジニアの前勤めていた会社や他の会社ではレビューの文化があまりないと聞いたので、この文化は嬉しい点の1つです。
早速振り返っていきます。
6月 ~ 7月 右も左もわからない期
インターンなどでRailsには触れていたもののやはりまだわからないことが多くとにかく手探りにコードを書いていました。
1. 不必要なJSON
JSONで返す際、`Phrase`モデルの全てのカラムが返ってしまうので内容を明示的に指定するべきです。
おそらく自分自身テストしたとは思うのですが、期待通り動いていることに満足してしまい、どういうJSONが返ってきてどこを使ってレンダリングしているのかまでは追えていなかったんだと思います。無駄なカラムは渡すべきではありません。
2. rubocopから逃げるな
メソッドが肥大化するとrubocopに怒られますが、その時は単に`disable`コードをつければいいと思っていました。
このレビューを受けてロジックを簡略化したり関数に切り出すことで、コードが読みやすくなったり不必要なクエリを産まなくなりスマートに書けるようなった気がします。**rubocopに怒られなくてもその癖がついたのが良かったです。**
3. マジックナンバー
とにかくenumを使うのを忘れていたり、マジックナンバーを多用していました。コードを書いて期待通りに動くことに必死だったのでリファクタリングまで全く考えが回っていなかったのだと思います(今もその傾向はあるので早く直したいです...)
8月 ~ 9月 ちょっと成長した期
初めて大規模な機能の実装を担当して成長できました。
4. どこに書くのが良いのか
まだどの部分がどういう役割なのか判断がつかなかった頃。今見ると気持ち悪さを感じます。マジックナンバーも直っていない。
5. そのコード、そのモデルで良いの？
MVCのどの部分がどういう役割なのか理解ができたのですが、果たしてそのモデルでそのメソッド書くべきなのかが判断できていなかったです。
6. そのメソッドの意味、わかってますか？？
メソッドの意味や用法を詳しく理解せずに使っている時がありよくコメントを貰っていました。期待通り動くものの冗長な書き方をしている場合が多く見受けられます。
10月 ~ 11月 ある程度書けるようになった期
ある程度書けるようになって書くスピードもあがってきました。
7. 条件の順番
ifの条件の順番についての指摘です。
左から処理が走るのでA ⊇ Bの場合は`B && A`と書くよりも`A && B`と書いた場合、AでfalseであればBのクエリを発行する必要がなく落ちてくれます。「確かに」と思いつつそこまで意識できていませんでした。
まとめ
以上いくつかもらったレビューを踏まえて、自分の問題を2つに分類できると思いました。
パフォーマンス面
これはメソッドの理解や無駄なクエリを発行しないなどが当たります。
無駄なクエリや必要のない重い処理がどんどんと積み重なるとサービスにも影響を及ぼします。ただ動くだけではなくやりたい子に最適なコードを書けるよう、知識の幅をどんどん広げるべきだなと感じました。
言語の理解を深めてよりスマートなコードを書けるようになりたいです。
リーダブル面
ここにはマジックナンバーやenumで書くことだったり、メソット内の処理をprivateメソッドなどに切り出すなどが入ってきます。
これは言語の理解以前に習慣や癖をつけることでレビューで突っ込まれなくなると思います。常にリーダブルなコードを書いてきたいです。
コードレビューをまとめてみて、全体的にまだまだ期待通り動くことに満足してしまっていると感じたので更にスマートなコードを書けるようになりたいです。