m_yamamo04172009-09-08

お世話になっております。

社内の人間が実装したクラスを使ったときの話です。
メソッドが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など。プロジェクトの規約に合わせて。

*2:Effective Java 第2版の「例外」あたりをご参照ください。