コードレビューって難しい?
1. はじめに
こんにちは!エンジニアの井上です。
最近は日曜になるとヒトカラ行ってます。それだけです。
新卒で入ってもうすぐ1年が経とうとしています。入社してすぐに仕事用に買ったキーボードはいろんなところがテカリはじめました。
母音のAIUEOよりもNがテカってる理由はよくわかりません。方向キーはテッカテカです。
いろんな仕事を任せてもらえるようになり、最近では技術リーダーの方から「練習でコードレビューやってみよう!」ということで、カスタマイズ案件でちょっとやっていますが、これがなかなか大変だったのでそのお話です。
2. コードレビュー
コードレビューって?
まあ、読んで字の如しなんですが、開発や修正したものをソース管理に反映させる前に他のエンジニアにおねがいして様々な観点からチェックを行うことです。
レビューではプログラムとしての品質を担保するために様々な観点から指摘を行います。
・コード重複してないか?(DRY)
・コード規約に準拠しているか?(命名規則、インデント、改行の位置etc)
・不要な処理が書いてないか?
・セキュリティ意識できているか?
・そもそもバグってないか?
などなどです。
w2で販売している「w2Commerce V5」はすべて認定レビュアーによるレビューを通過したソースコードになっています。
様々な観点
ここ数ヶ月、とある案件の追加開発、保守開発にアサインされているのですが、技術リーダーの方から「練習でレビューやってみてよ!」と言われたので、とある同期のコードに関しては私がレビューすることになりました。私のコードはその同期にレビューしてもらっています。
これが意外とむずかしくて…
そもそも製品のコードを高いレベルで理解していないと指摘が出せません。
「ここでやってる処理、〇〇クラスのアレが使えるんじゃない?」みたいなのは、知っていなきゃわかりません。
他にも、「ここを改修したならここに影響がでる」というのもわかっていないとバグを埋めることになってしまうので、もちろん開発者の責任にもなりますが、コードレビューの段階で気付けるならもちろんそっちのほうがいいです。
あとはセキュリティです。
簡単なところで言えばHTMLエンコード(サニタイズ)漏れです。これを見逃すとXSS脆弱性に繋がります。
パフォーマンス面も気をつけなければなりません。開発環境ではわからなかったが、本番環境で動かしたらWebサーバがパンクした…なんて事にならないよう、特にエンドユーザーが触れるフロント画面については細心の注意を払い、問題があれば指摘しなければなりません。
やってみてどうだった?
結論からいえばとても大変です。
コード規約レベルなら当然指摘できますが、セキュリティ面や考慮漏れなどはなかなか気づくことができません。
やってみてよかったのは、逆に製品知識がついてきたことです。
みている中でここってほかはどういう実装かな?と、色々見ることで理解が深まりました。
3. まとめ
私はまだ認定レビュアーではないので非カスタマイズ案件(標準パッケージ)の方はコードレビューできないのですが、今後認定してもらえるようにコードを書く力だけではなく読む力を養わなければなりません。
認定レビュアーになれるよう、今後も精進していきます!