コードレビューの視点 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言語に限らず、通常のデバッグ文やログ情報にも同じことが言えます。せっかくデバッグ文やログ情報を残すようにコードを書いても、いざ、デバッグ時に必要な情報(変数やポインタの値など)が出力されていないと何の役にも立ちません。
デバッグ文やログ情報出力は、コードの可読性を下げる場合が多いです。しかし、それでも必要があると判断して記述するのであれば、デバッグに必要なエラー情報はすべて表示したり記録したりすることが重要です。コードレビューをしていると、その点を全く考慮せず、デバッグ文やログ情報出力を書いているコードを多く見かけます。