カシオの新人がコードレビューで受けた指摘理由まとめてみた（後半）
はじめに
こんにちは！ 今年で入社2年目の元新人です。
この記事はこちらの続きとなります。詳しい内容はこちらをご覧下さい。
前回はトップ10のうち10~6位までを紹介していましたが、今回は5~1位の紹介と、比較になります。
5位 判定方法が違う （13回）
ロジックの判定の数式が条件を満たしていなかった、などのミスです。
nullチェックなどがうまく行っていないなども該当します。
実装前からわかるような内容が多いので、あらかじめ条件を整理しておけば防げるものもあったかと思います。
特に異常系を考慮しきれていないので条件に抜けがあることが多かった印象です。
4位 アプリ構造の理解不足 （15回）
既存アプリの変更に携わった、というのもあるかと思いますが、アプリの構造が理解できていなかったことによるミスです。
例えば以下のようなミスです。
本来別の場所で処理する内容を、不適切なところに書いてしまっていた
値渡しの方法が間違っている
経験不足もあるのでしょうが、一般的な設計方針、方法を身に着けて行けば正解がわかるようなケースもあったと思います。
3位 変更箇所の消し忘れ （18回）
ある場所を変更した際に、変更前の参照されなくなったコードを消し忘れてしまったことが原因です。
これは、とりあえず動くようにして確認しよう、と実装してみたコードの精度が低いのでリファクタリングで大幅に修正することになるのが原因だと考えています。
そもそもとりあえず動くようにするではなくある程度先に方針を固めてから実装する、検証用と本実装のブランチを分けておく、変更差分をしっかり確認する、などで防げるかと思います。
2位 命名規則 （27回）
関数名、定数名が不適なので修正するような初歩的なミスです。
キャメルケース、スネークケースに則っていない、（初期の頃は）使い分けがわからないなども含みますが、単純に関数や定数の役割と名前があっていないケースの方が多かった印象です。
他には以下のケースがありました。
途中で名前を変更した際に単語だけ変更して言葉として成り立たなくなった
例：getStringTimeのgetを他の箇所で使われていたformattedに変更
→formattedStringTimeになってしまう（Stringが不要）
冒頭の言葉を消したのでキャメルケースに当てはまらなくなった
例：initialValueのinitialが不要なので削除したところ、
Valueになってしまい、最初が大文字になってしまう
英語力があまり無いので、正しいのかが直感的に分かりづらいのも問題だったかもしれません。
1位 可読性 （36回）
わかってはいましたが、最も多いのは可読性が悪いので改善してほしいといった指摘です。
ひとまとめに可読性、としてしまうとイメージしづらいので多かった理由を以下にまとめておきます。
関数の構成的に、無意味に複雑な処理、回りくどい処理をしてしまっている
例：オブジェクトで格納すれば簡単な部分を無理に配列で管理しようとして値の管理が複雑になる
一文が長すぎるので改行する、分割するなどで短くしてほしい
例：if文などの条件が複数かつ変数名が長い場合は、条件一つ書いたら改行する
if文などでむやみにelseを使わずに早期にreturnやcontinueするようにしてネストを浅く保つべき
例を下記に示します
// 指摘前
hogeHoge () {
if (a === b) {
calcValue(c);
} else {
calcValue(c-d);
}
}
// 指摘後
hogeHoge () {
if (a === b) {
calcValue(c);
return;
}
calcValue(c-d);
}共通化できそう、他の処理でも使用できそう、関数が長すぎるなどの理由で、関数を共通化したほうが良い
経験の部分も大きいと思います。特に実装する部分だけ見て修正や追記を行うと、コード量が多くなりがちかと思いますので、俯瞰で関数を見ていくことで減らせるのではないかと感じました。
生成系AIなどを使うと逆に一文を長くしたり、早期にreturnやcontinueしないような処理になる場合もあるので、チェックを欠かさないよう、心がけていきたいと思いました。
全体を通して
円グラフにしてみて、比較してみます。
項目すべて記載＆全てに色分けしたので少し見づらいかもしれませんが、
温かい目で見てもらえると幸いです。
一覧にしてみるとわかりやすいですが、単純なミスが多かった印象です。
可読性に関する部分は自分でも課題と思っていましたし、多くなるのもわかっていましたが、命名規則が思っていたのよりも多いことには驚きました。
ネーミングセンスを磨いていきたいと思います。
あとは、変更消し忘れに気づかないことも多かったです。
プルリクエストを出す前に差分は確認するので、消し忘れは比較的見つけられますが、不要になった部分を見落とすことが多いので意識してチェックするようにしたいと思います。
最初三ヶ月と直近三ヶ月で受けた指摘理由の違い
最初三ヶ月に関して
JavaScript、HTML、CSSに初めて触れました
プルリクエストも初めて出しました
後半は簡単な部分に関してレビューする側も行いました
おかしな仕様は可読性以前に動きとして破綻しているものが対象です。
最初の方なので変更消し忘れや、理解不足系が比較的多くなっているのがわかります。どれかが偏って多い、というよりは満遍なく指摘されている印象です。
直近三ヶ月に関して
後半は生成系AIを使うようになりました
最初三ヶ月と比べて、理解不足系や変更消し忘れなどが減ったので、着実に成長しているのでは？と感じられますが、命名規則は直近になっても改善していないことがわかりました。
また、思ったより多いスペルミスの理由に関しては、上記でも書いた桁数のミスが響いたのだと思います。
おわりに
ここまで読んで頂き、ありがとうございます。
個人としては一年目の振り返りをあわせてできたので、とても良い経験になりました。
また、思ったよりも初歩的なミスが多いことには反省して、再発防止に努めたいと思います。
私のミスが記事になるのは少し恥ずかしい気もしますが、この記事を読んで皆様のレビューの負担が少しでも軽くなったら幸いです。
【リンク】カシオのソフトウェア開発について