■
お世話になっております。
社内の人間が実装したクラスを使ったときの話です。
メソッドがpublicになっていたので呼んでみたらNullPointerException。よくある話です。
ソースを読んでみたら、おおよそ以下のような感じでした。
package do.not.use; class BadClass { Object a, b, c; public int enter(){ a = new Object(); int retA = methodA(); int retB = methodB(); return retA + retB; } public int methodA(){ b = new Object(); if(a.equals(b)){ // NullPointerException発生のおそれ return 0; }else{ return -1; } } public int methodB(){ c = new Object(); if(b.equals(c)){ // NullPointerException発生のおそれ return 0; }else{ return -1; } } }
私なら以下のように直します。
- インスタンス変数の可視性をprivateにする。
- 可能であれば、methodAとmethodBの可視性をprivateにする。
- 上が不可能(依存するクラスがある)なら、methodAおよびmethodBの入口でそれぞれインスタンス変数a、bがnullでないか確認し、nullであれば適切な例外*1を発生させる。
- インスタンス変数にはthisをつける。
今回のケースはソースを読めたので対応できましたが、そうでなければ完全に手詰まりでした。
ソースを読めばわかるのは当たり前。しかし、人のソースを読むのはやはり時間がかかります。
なるべくソースを読まなくても使えるようにするには、パラメータやインスタンス変数をチェックし、適切な例外*2を投げるようにするのが一番だと思います。
以上
*1:IllegalStateException、NullPointerException、RuntimeExceptionなど。プロジェクトの規約に合わせて。