コードレビューの視点 002

必要なエラー情報を付加する

何らかのエラーを検出した場合の対処方法としては、いくつかあります。

• 公開APIが呼ばれた場合のメソッドの引数の値が不正だった場合に、NullPointerExceptionやIllegalArgumentExceptionをスローしたりします。
• デバッグ用にデバッグ文を表示したり、ログ情報を記録したりします。

コードレビューを行っていて気づく点としては、これらのエラーを通知してデバッグの助けとして書かれているコードが全く何の助けにもなっていないことが多いということです。

Javaでは慣例として、引数のnullの場合には、NullPointerExceptionをスローします（『Effective Java 第2版』 「項目60 標準例外を使用する」）。その場合、Javadocにその旨をきちんと記述すると同時に、例外をスローする際にエラー情報をコンストラクタのメッセージとして渡す必要があります。たとえば、引数valueがnullであってはいけない場合には、次のようになります。

throw new NullPointerException("value is null");

この場合、引数なしコンストラクタを使用すると、例外がスローされた場合に引数の誤りなのか、呼び出された側のバグなのかの区別が付きません。

引数の値が不正であった場合も同様です。たとえば、valueが負の値ではいけない場合には、次のようになります。

if (value < 0) 
    throw new IllegalArgumentException("value(" + value + ") is invalid");

コードレビューを行っていると、この場合に、IllegalArgumentExceptionの引数なしコンストラクタが呼ばれていたり、あるいは単に"value is invalid"と書かれていたりすることが多いです。実際にデバッグをする段階になって、どのような値であったかを知る必要があると、「値を表示するようにコードを修正して、障害を再現させて値を確認する」という行動が必要になります。

Java言語に限らず、通常のデバッグ文やログ情報にも同じことが言えます。せっかくデバッグ文やログ情報を残すようにコードを書いても、いざ、デバッグ時に必要な情報（変数やポインタの値など）が出力されていないと何の役にも立ちません。

デバッグ文やログ情報出力は、コードの可読性を下げる場合が多いです。しかし、それでも必要があると判断して記述するのであれば、デバッグに必要なエラー情報はすべて表示したり記録したりすることが重要です。コードレビューをしていると、その点を全く考慮せず、デバッグ文やログ情報出力を書いているコードを多く見かけます。