情報システム特別講座 :

アプリケーション開発で質の高いコードレビューを実現するためのポイントとは ? ~ 後編~

2020-10-01

How to be a Developer

森崎修司
名古屋大学大学院情報学研究科准教授

はてブ »

前編で『コードレビューの目的』や仕様との整合性、読みやすさを例とした『コードの読み進めかた』を説明してきました。後編では、ネットワークを経由して呼び出す API に関わる読み進めかたの例を説明します。

Web API の呼び出し

ネットワーク経由で呼び出す API はこうした要素技術の代表的なものの一つです。通信が不安定になる場合がありますので、送信できなかったときの例外処理や再送信といった処理が必要になります。こうした処理が実現できているかどうかを確かめるための読み進めかたは次のように書けます。

確かめる箇所

API 呼び出しの前後

確かめかた

送受信できなかった場合の例外処理や再送信のための処理があるかチェックする

AWS SDK が提供している ClientConfiguration クラスのように、送信できなかった場合に、再送信を実施してくれるライブラリもあります。また、再送信が連続してうまくいかない場合には、再送信する時間の間隔を伸ばしながら再送信 (エクスポーネンシャルバックオフ) してくれます。こうしたライブラリを利用しているかをチェックしてもよいでしょう。

API 固有の問題が起きないかを確かめるもう一つの例として、データベースを検索するAPI の前後の読み進めかたを紹介します。こうした API では、検索条件によっては検索結果が多数になり、一度のリクエストでは検索結果の全部を受け取れない場合があります。また、複数回に分けて検索結果を受け取るときには、これまで検索結果のどこまでを受け取っていて、どこからを受け取りたいかを API 側に指定しなければならない場合があります。これらをチェックする方法は以下のとおりです。

確かめる箇所

データベース検索 API 呼び出しの周辺

確かめかた

受信した検索結果がすべての検索結果か、一部か (続きがあるか) をチェックしているか確かめる。また、複数回に分けて受け取るしくみがあるか確かめる。

たとえば、AWS の Dynamo DB が用意している API である BatchGetItem は、検索結果が 100 件か容量が 16MB のどちらかに達すると結果の一部しか返されません。そうした場合が考慮されたコードになっているか確かめます。

まだ返されていない検索結果は UnprocessedKeys として戻されますので、検索結果を受け取るときには複数回に分けて受け取っているか、そして続きを受け取るために UnprocesseddKeys を API に渡しているか確かめます。

効率化を目指そう

読み進めかたを改善していくと効率化につながります。改善の方法の一つは読み進めかたを整理したり、バリエーションを増やしてコードレビューの対象に応じて適切なものを選んだりできるようになることです。他にも、一部を自動化して目視で確かめる箇所を減らす方法もあります。

読み進めかたの整理

読み進めかたを整理することはいつでもできるのですが、何かきっかけがあったほうがやりやすいです。きっかけとして、リリース後やレトロスペクティブをお勧めしています。どういう問題を見逃していたか、他のレビューアからどういう指摘があったかを振り返ります。見直すのはめんどうといった理由で消極的になりがちですが、見逃していた問題や他のレビューアの指摘を振り返れば、気づきがあります。

一般化できるものは一般化して整理していきます。読み進めかたが増えていくと分類したくなるのですが、あまり詳細な分類はお勧めしません。分類には様々な視点があるので、きれいな分類は難しいからです。

今回紹介したもの以外にも、インタフェース定義の不備、条件分岐の不備、推奨されている共通ライブラリやミドルウェアの不使用、リソースリーク、コーディングルールの違反、課金対象(キャパシティユニット)となる API 利用が妥当かといった具合に、様々な視点からの読みすすめかたが考えられます。細かい分類を作っていくよりは、読み進めかたをグループ化する程度にしておきましょう。

自動化

効率化につながるもう一つの方法は自動化です。コードレビューには時間がかかるので、これまでにも様々な自動化が試みられています。古いものだと、コンパイラが出力する警告 (warning) があります。コンパイラの実行結果に、使われていない変数があることを警告するメッセージを見たことはないでしょうか。警告では構文解析でわかる不自然な部分を出力します。

先に挙げた switch case の break を確認する読み進めかたの場合、構文解析によって確かめる必要のある箇所を自動的に特定できます。あとは、その箇所を目視して、意図したものかそうでないかをチェックするだけです。 break のない case があるかどうかすべての switch case を目視でチェックする方法と比較すると効率化ができます。また、コードレビューでチェックするのではなく、コードを書き終えた時点で書いた開発者がセルフチェックできるようにすることでも、効率化につながります。

また、FindBugs (SpotBugs) のようなチェックツールや Eclipse のような IDE の警告を利用することも効率化につながります。下のコード (図 2) は、そうした例の一つで、スレッドセーフでない部分が警告されます。

下のコードは、Java のクラスライブラリの ConcurrentHashmap を使っていますが、スレッドセーフではない部分があります。ConcurrentHashmap は、キーバリューストアの実装の一つです。複数のスレッドから同時更新されると結果が保証されません。

下のコードのように、containsKey() でキーがあるかどうかを確かめてから、get() しようとすると、containsKey() を実行したときにあったキーが別のスレッドで削除されると、次の行の get() を実行するときに、なくなっていることもあります。この部分がスレッドセーフではないと警告してくれます。

図2: スレッドセーフではないコードの例

public class Configuration{
   private ConcurrentHashMap <String, String> repository; 
・・・
public String get(String key){
if (this.repository.containsKey(key) {
    return this.repository.get(key);
}
return null;
}
・・・
}

これまで、こうした警告の多くは事前に定義されたルールによって、該当箇所を指摘するというものが多かったのですが、最近では機械学習を使った新しい技法が使われ始めています。

一つ目は、Github のような大規模なソースコードリポジトリを学習データとして、API の呼び出し順序や構文木の一部を機械学習モデルに学習させ、問題があるかもしれないコードを予測させる方法です。ほとんどのコードでは使われていない API 呼び出し順や修正履歴のある構文木とパターンマッチする部分を警告として提示します。こうした機械学習モデルを活用して問題点を予測する技術は、「Big code」と呼ばれています。

機械学習を使ったもう一つの機能は、問題があるかもしれない部分の警告や指摘の精度向上です。コンパイラの警告、ルールベースによる潜在的なバグの指摘、Big code による予測のいずれも誤検出 (実際には問題がないのに問題箇所として警告、指摘してしまう : false-positive) を含みます。そうした誤検出を減らすために、警告や指摘をみた開発者がそれらが役に立つかどうかをフィードバックすることで、警告や指摘が役に立ったか学習していきます。役に立つというフィードバックがあった順に表示したり、そうしたフィードバックがなかった警告を表示しないようにしたりします。

Amazon CodeGuru では、役に立ったかどうかのフィードバックをサービスとして受け取ることができます。利用者みんなでフィードバックを与えて、育てていくことができます。また、スレッドセーフでない部分やリソースリークの可能性のある部分の警告してくれます。