• Home
• コーポレートブログ
• 他人の書いたコードを読むこと（コードリーディング）は楽しい

原田です。4月になりやっと春らしくなりました。会社からの帰り道の川沿いに咲いている桜も満開なので、週末はお花見を楽しむ方も多いかと思います。今回はコードリーディングについて書かせていただきます。

ほかの人のコードを読む

前回のブログにも書きましたけど、オープンソースのプロジェクト管理ツール「Redmine」の改善を業務として行なっています。この改善業務ではプログラムコードの作成や修正をしてRedmine公式サイトに投稿します。投稿したパッチが採用されRedmine本体に取り込まれたものは、新バージョンのRedmineで新機能や修正として世界中のRedmineユーザーに利用されています。先日リリースされた新バージョンのRedmine 4.0.3 / 3.4.10でも当社作成のパッチ8件が採用されました。

当社ではRedmine改善業務を数名で取り組んでおり、それぞれ異なるRedmineの改善に取り組んでおります。自分で書いたコードだから正しく書けていると信じたいのですけど、やはり第三者の目から見て適切なコードであるか判断をしていただく必要があります。そのためRedmine公式サイトに投稿する前に、各自作成したコードをほかの人がコードレビューをします。

コードレビューにルールを設けています

「私が作成したコードをほかの人にコードレビューしてもらう」ということは、その逆の「ほかの人が作成したコードを私がコードレビューする」ことにもなります。私がコードレビューする時は自分なりのルールを3つ決めております。

ただしあまりにも細かい点を指摘しすぎるとコードレビューを依頼する人（レビューイ）のヤル気が失せてしまう恐れもあるので、最後の2点に関しては「個人的に気になったこと」として返信し軽い気持ちで見てもらうよう心掛けています。

処理の誤りは指摘する

これは当然ですけど、Redmineの改善要望で挙がっている内容に沿って作成・修正が行われていることを確認します。

メソッド名や変数名の正確性に関しては指摘しない

コーディングした名称の意味合いが大きく誤っている場合は指摘しますけど、基本「変数名（XXX）はYYYであるべき」のような指摘はしないようにしています。これは（過去に）ある事案の設計レビューに参加した時、書面上の誤字脱字レベルの指摘で時間の大半を使ってしまい内容の薄いレビューになったことを経験したためです。

コーディングルールに関しては指摘しない

コーディングルールが明確に決まっている場合は指摘しますけど、基本「後置ifの空白」「インデント幅」「alias定義があるメソッドはどれを使うべきか（ちなみに私はcollect,inject派で、map,reduceは好んで使いません）」などの指摘はしないようにしています。個人的には上記名称の正確性と同じレベルの指摘で、レビューとしてはさほど意味が無いものと思っております。

様々な人がRubyコーディング規約を決めようとしているのは知っていますけど、Redmineに関しては2006年から世界中の人々により開発が進められており、たぶんコーディング規約のようなものは無いと思います。実際のコードを眺めてみると分かりますけど、インデント幅がスペース2つ（Rubyで一般的に使われるインデント幅）のものもあれば4つのものもあります。

初めて知ることも多々あります

私はこれまでRubyを含む様々なプログラム言語を業務で扱ってきました。それなりに経験はあると思っていましたけど、他人の書いたコードを読むとあらためて初めて知る関数（メソッド）・ロジック（処理の手順）などに遭遇し刺激を受けます。これが私のコードリーディングをする楽しみの一つでもあります。先日Redmine改善業務を行なった際に String#b メソッドの存在を初めて知り感心しました（Patch #31059: Use #b shortcut instead of #force_encoding）。

レビューアとしていろんな知識を得ることは大切です

レビューを依頼される人（レビューア）の中には、単にコードを読んでOKかNGかを判断する方もおられると聞きます。しかしレビューを依頼する人（レビューイ）はOK・NGの返答だけがほしいのではなく、レビューイ自身がスキルアップするためのコメントや指摘などを求めていると思います。

そのためにはレビューアも新しいことを学ぶ必要があります（ITの世界は進化が速いので追いつくのは大変ですけど）。偶然ですが、先日当社で定期購読しているWEB+DB PRESSでRuby 2.6に関する機能紹介の記事を読む機会がありました。Rubyは古い書き方もサポートされているので2.6になったからと言って大きく変わることはないと思っていましたけど、範囲（Range）の終端が省略できることを知って感激しました（Feature #12912: An endless range `(1..)`）。知らないよりは知っていたほうがコードレビューする時に役立つとあらためて認識しました。