読者です 読者をやめる 読者になる 読者になる

アナログCPU:5108843109

ゲームと音楽とプログラミング(酒と女とロックンロールのノリで)

コーディング完了報告する前のセルフレビューチェックシート

会社やプロジェクトによると思うので、まあ、見る方は参考までに。

参考…というかこちらに触発されて書いています。mynavi-agent.jp


まあ、基本的に確認したい内容は同じではあるのですけどね。
ただうちはちゃんとしたコードレビューなどというものはなく自分でやりきるしかないので、このチェックで最終になっちゃいますね。
(まあレビューがあるとしてもレビュー任せはよろしくないので、完了のつもりでチェックすべきですが)

あと集約するとやっぱり

このソースコードは美しいか。
本人が美しいと思っていないソースコードは、レビューに出しちゃダメである。戦え。

これに尽きますね。これを突き詰めるためのチェックと言ってもいいですね。

セルフレビューチェックシート 9項目+α

(何か思いついたりやらかしたりしたらまたこっそり増えるかもしれない)

□ 仕様をすべて満たしているか(過不足がなく、正しい動作をしているか)
□ あらゆるパターンを考慮して動作確認を行ったか
□ 表に見えない部分でエラーやWarningが発生していないか
□ コーディング規約を守っているか(改行やインデント、変数関数の命名規則など)
□ 変数や関数は用途を正しく理解できる命名になっているか
□ 事実と異なるコメントはないか
デバッグ用の処理やメモコメント等の不要コードはないか
□ シンプルで無駄のないコードになっているか
□ 処理速度はもっと上げられないか

□ 一息ついたらもう一度最終確認(日を改める余裕があるならそれがベスト)

解説という名の自戒

上記項目は、ざっくり以下のカテゴリに分けられます。

  • 仕様通りか・不具合がないか
  • ルールを守っているか
  • 可読性に気を付けているか
  • コストに気を付けているか

もちろんすべてクリアするのが理想的ですが、作業時間には限りがあるので優先度を決めてチェックすべきでしょう。
上記では大体優先度順に書いたつもりですが、何を重視しているか、またどんな機能なのかによっても異なってはくると思います。
例えばユーザの入力に影響されない関数であれば例外処理をガチガチに固めるよりも処理速度を上げる方を優先する必要があるかもしれませんし、
1回きり使い捨てのプログラムを数多く書くような現場であれば可読性やコーディング規約は重視されないかもしれません。


<仕様通りか・不具合がないか>

いずれにしろ、これは大前提ですね。

求められている仕様が揃っているか。
求められていない機能を勝手に入れていないか。
システム開発のプロではない相手から提示された仕様は大概例外的な操作を含むパターン網羅なんてされていません。「適当によろしく」みたいなセリフも信じてはいけません。というか上流工程できちんとした仕様書になっていてさえ何かしら抜けはあるものです。
仕様書(という名のメモ書き)にしろ仕様書(ガチ)にしろ、不明な点については必ず確認した上で、提示された通りの仕様で実装します。

また、様々なパターンを考慮し、最低でもすべての分岐・繰り返しを通るテストを行います。
基本的なテストは漏れなく行います。
(テスト技法についてはここでは詳しく触れませんが、同値分割・境界値分析や、例えばWEB系ならHTMLタグなど異常値の入力、NULL値・空文字・ゼロの判定など)
また、例外的なパターンを多数試すのに夢中で通常パターンのテストが甘い、などという本末転倒なオチにならないよう注意します。
実際に使用するデータで試すことができるようならそれがベストですね。
テスト用に一時的にデバッグ表示などを入れていた場合は、消した後、もう一度動かします。
また、表に見えない部分も忘れずチェックします。
ログファイルや、WEB系であればcookieやセッション、キャッシュの挙動、JavaScriptのエラーなど。
見た目は正常でも、裏で大量のエラーが…などということもよくあります。


<ルールを守っているか>

改行やインデントの位置は適切か。
変数や関数は命名規則に従っているか。
関数ヘッダ(関数の前につける説明コメント)は書式通りか。
ルールを守ることは、不具合の防止や可読性の向上にもつながります。
コーディング規約を明文化していない会社や現場もありますが、その場合は他の人のコードに合わせた書き方をするよう心がけます。
(明らかに周りが変であっても勝手に変えず、改善の提案をして合わせてもらうよう働き掛ける)
例えば「○○かどうか」を判断する系の関数で、戻り値が統一されていない(true/false系と1/0系が混在しているなど)と、使うときにいちいち確認しなければなりませんし、可読性は落ちますし、不具合の原因にもなります。
形式だけでなく、ライブラリ等に既存の処理を自分で別途作ってしまったりしていないかなども要チェックです。


<可読性に気を付けているか>

他人が読んで理解しやすいコードになっているか。
またそれ以前に、自分で理解しているコードになっているか。他人に説明できるか。

自分で理解できてないコード、意外とあります。だいたいはコピペのせいです。
既存のコードやグーグル先生の回答からのコピペで済ませるのはプログラマにとってとても重要なことでもありますが、仕組みを理解して使うのが大前提です。何かあっても自己責任です。
(わたしが時々うpしているのも決して無条件に信用してはなりません…わざと嘘を書いたりはしませんが素で間違えていることもあります…)
標準関数や一般的なライブラリの中身まで見る必要はないと思いますが、そういうものも極力マニュアルは見て使うと良いと思います。

読みにくいコード・他人に説明できないようなレベルのコードは、他人だけでなく数ヵ月後数年後の自分の首を絞めることもあります。
極力シンプルなロジックになっているか。
サブルーチンの分け方は適切か。
変数や関数の命名はわかりやすいか。勝手な略語を使っていないか。統一性があるか。
コメントがきちんと書かれているか。そのコメントは正しいか。余分でもないか。
「読みやすい・読みにくい」は個人差があるので突き詰めても正解はありませんが、最低限は気を付けていきたいところです。


<コストに気を付けているか>

無駄な処理は見た目に邪魔なだけでなく、当然処理速度などが落ちる原因にもなります。
怪しい部分は面倒くさがらずにきちんと検証を行い、よりよいコードを残したいものです。