自主的20%るぅる

各々が自主的に好き勝手書くゆるふわ会社ブログ

コードレビューの観点

新卒二年目に先輩のコーディングレビューしてた筆者が語るレビューポイント

こんにちは、にんじんです。

「一度つけてみたかった」という理由で煽り要素満載のタイトルとなっておりますが、内容はそんなに煽ってないはずです(笑)

突然ですが、みなさんはコーディングをする際 または コードレビューをする際にどのような部分に着目して実施していますか?レビュー観点やコーディング規約が設定されている場合もありますが、プロジェクトによっては存在しないことも多いですよね。

ということで、今回はそのあたりについて書いてみようと思います。

「今まで意識したことなかったな……」という方に、この記事が参考になれば嬉しいです。

前提条件

私自身、ここしばらくはJavaの案件にばかり携わっているので、Java前提で書き進めていきます。

もちろんJava以外の言語においても適用できるようなことを書くようにしましたが、マッチしなかったらすみません!1)javascriptは+αで言いたいことがありますが、それは別の機会に譲ります。

それでは──語ってまいりましょう!

些細なミス編

油断大敵 誤字脱字

日本語で書かれたコメントに潜んでいるのをよく見かけたりしますが、英語で書かれた変数名やメソッド名などにも潜伏したりしているので油断大敵です。

スペルが間違っていようがコンパイルが通れば動いてくれるのがプログラムというものですが……。
誤字脱字があると、やっぱり不格好ですよね。

他の人に「あ、間違えてる!」なんて思われるのもツライですし、自分で自分のミスに気づいた瞬間はもっとツライ。

Javaなどのコンパイルを必要とする言語であればエラーとして検知できることもありますが、スクリプト言語などでは「なぜか動かない……」なんてこともよくある話です。

最初にキーボードを打ち込む瞬間から気を付けておきたいところですよね。
(共通処理に残る永久に消せないスペルミス、どうやって無くそうかな)

嘘つかないでね コメントくん

個人的な意見としては、「少なくともJavaで言うところのJavadocくらいのコメントくらいはクラスやpublicなメソッドにつけておいて欲しい」と考えています。

いろいろな意見があるのは承知していますが、ほとんどの方は何かしらコメントは書くだろうと言う前提で話を進めます。

ソース中のコメントを読んでいると、「アレ?このコメント、嘘ついているな……」なんてのを見かけるのは良くありますよね。

「書いてあることと処理が全然噛み合わない」とか「仕様変わってコード書き直したのにコメントそのまま」とか……。

本来よりどころとなるはずのコメントがまったく信用できないというのは哀しいことです。
(もう「嘘つくくらいならコメント書くな!」って感じですよね)

使われてないなら消してくれ

思い通りの処理を実現するために試行錯誤を繰り返していたりすると、不要な変数やメソッド、クラスやコメントアウトされたコードが残ってしまう……なんてのは良くある話です。

ほとんどの場合はgitなどでバージョン管理をしているはず2)うちはまともなバージョン管理してないってのは無理 of 無理なので認めませんなので、不要なコードはすべて消しておいて欲しいものですよね。

「あとから使うかもしれない」という考えは「ずっと着てない服を捨てられない」人と同じ。使われなくなったら、いったん破棄したらよいのです。

バージョン管理していれば後でサルベージできますし、必要ならさらによいロジックで作り直せばよいのです。
(このロジックバグってるじゃん!と思って直したら使われてなかったことがありました。ツライ。)

プログラム構成編

守っておくれ そのお作法

ある程度の規模のシステムなら、何かしらのフレームワークを使っていることでしょう。

例えばJavaなら「Spring Boot」などですね。

フレームワークごとにさまざまな「お作法」があるかと思いますが……。

Spring Bootで実装するときに気をつけて欲しいポイントベスト3~構成編~

 

そのお作法、ちゃんと守ってますか?

「正常系だと上手く動くんだけど、レアケースの異常系で死ぬ」なんてやっかいなことも、お作法に沿っていれば発生しにくいものです。

お作法は大事です。とてもとても大事です。
(大事なことなので2回言います)

もちろんシステムのコーディング規約がある場合は、それも守りましょう。
(めんどくさいけど、ルールを守ることが自分を守ることになるんやで)

同じロジックはもうウンザリ

「流用新規」。

大きなシステム開発を行う際によく聞くワードではないでしょうか。

私も何度もやったことがあるのですが、それ本当に「複製」で大丈夫ですか?

スケジュールが押しているのもわかります。コピーすれば話が早いのもわかります。

でも、何も考えずに複製を重ねていけば、システムが肥大化の一途を辿ってしまう原因にもなりかねません。

グッとツラさをこらえて、共通化しましょう。そうすれば、きっと未来の誰かがラクになるはずです。
……とはいえ、スケジュール上あまりに苦しい場合はさすがに諦めますけどね。3)FIXMEでもつけて未来の誰かに委ねましょう
(明らかに何も考えてなさそうな複製は問答無用で共通化させるけどな!)

プロジェクトで大事なこと編

その修正 あの場所にも必要かも

「横展開」。

これもよくあるやつですよね。

そもそもの話、ちゃんと共通化されていれば不要な作業なのですが、しっかり設計した美しいシステムであってもある程度のコードの重複はあるものではないでしょうか。

「システムAとシステムB、仕様が共通だから両方のコードを直さないと!」なんてこともありますよね。

まだそれを把握して気づければ良いのですが、見落としてしまうこともあるわけで……。
今のプロジェクトはテスト部隊が別のチームなので、あとから別の場所の同じ不具合を見つけられたときは本当に切ないものです。

気づけないものはしゃーないけれど、他にも修正が必要か?と一呼吸おく習慣はつけたいですね。
(えーそこ共通化されてなかったん?みたいなことが日常茶飯事)

3ヶ月後のあなたは別人

そのコード、他人が読んでもラクに読めるコードになっていますか?

勢いに任せてコードを書くこともあるかと思いますが、書いた瞬間は仕様もバッチリ頭に入っているものですが……。
3ヶ月ほど経ってから読み返してみると、諸々すっかり忘れてしまっているのが常ですよね。

久々にそんなコードを触ることになった暁には、「正しく動いてるっぽいけど、何をやってるのかコードから読み取りにくい」なんてこともよく聞く話です。

そうならないためにも、「1メソッドのネストの数」や「メソッド内の行数」なんかがコーディング規約で決まっている場合もあるわけです。
そういうものがあればそれに従い、そういうモノがなければ「未来の自分が読めるコード」を心がけると良いかもしれませんね。

二度と修正しないことが保証されているプログラムならいざしらず、忘れた頃にやってくる保守やら追加改修やらのときにで泣くのは自分かもしれません。
そう思えば、「どうすればコードがわかりやすくなるかを考えて実装しよう」という気になりますよね。
(そういう手を尽くしても「過去の自分、何やってんねん」ってなったりしますが)

少しでもクリーンなコードが増えますように

いかがでしたか。

当たり前のことも書いたかもしれませんが、スケジュールに切羽詰まると忘れがちですよね。
このリストを横目に、私自身もコーディングやレビューを進めたいと思っています。

この記事がクリーンコードが増えることに少しでも貢献できますように。
みなさまも、素敵なプログラミングライフを!

注訳はこちら

注訳はこちら
1 javascriptは+αで言いたいことがありますが、それは別の機会に譲ります。
2 うちはまともなバージョン管理してないってのは無理 of 無理なので認めません
3 FIXMEでもつけて未来の誰かに委ねましょう
Let’s share this article!

{ 関連記事 }

{ この記事を書いた人 }

アバター画像
てるもと

大阪にて弊社サービスのFairgritの開発をやってます!
ブログのアイキャッチに自分で撮った写真を使うのがマイブーム。

記事一覧