堅牢なコーディングルールを策定する方法(2)

Java

いやー、なんか怖い人（笑）が見てるようだ。突っ込み激しそうぜよｗ

さて、前回言っていた「判断ロジック」についての答えは各自考えてみただろうか？ 各方面の反応を見ると「1〜4どれでしょうか*1」という問いにすり替わっちゃってる気がするけど、テーマは「1〜4を決めるロジックを考えてください」なんだ。書き方悪かったもしれんｗ まぁつまり、昨日のエントリの時点では1〜4どれでもないのですよ。判断ロジックがないので。

というわけで本題。アプリケーションレベルでのバグとは「実際の挙動が仕様と違うとき」ですね。挙動があるべき姿(=仕様)と違う時にそいつバグとするのが妥当です。そしてそれはコードレベルでも同じ基準で良いんじゃないでしょうか。

じゃあ、仕様とは何か。Javadocコメントですよ。Javadocがないとバグの定義さえもできないんです。だから口酸っぱくJavadoc書けとｗ*2

まぁつまり「Javadocに書かれていない挙動をしたらバグ」です。というわけでJavadocを明確に記述します。

ちなみに、今回は例示しなかったが、基底クラスやインターフェイスの記述も重要です。例えば、基底クラスのhoge()メソッドに@throws SomeRuntimeExceptionが宣言されていない限り、サブクラスのhoge()でSomeRuntimeExceptionを投げてはならんです。そのインスタンスをインターフェイス越しに見た時に見えるJavadocは、インターフェイスのものなので。そこに記述されていない「SomeRuntimeExceptionを投げる」という挙動をすることになってしまいます。

まぁそんなJavadocを記述すると、バグかが明らかになりますね。で、バグには2種類考えられます*3。

  1. 仕様バグ
    • Javadocに書くべきことが欠落していた、もしくは間違ったことを書いていた場合。
  2. 実装バグ
    • Javadocに書かれた仕様は正しく、それに対応する実装が間違っていた場合。

前述のSomeRuntimeExceptionのくだりで「仕様バグ」という判断をしたら、「インターフェイスに@throws SomeRuntimeExceptionの宣言を追加する」というfixをすることになります。「実装バグ」と判断したら「サブクラスでSomeRuntimeExceptionを投げずにどうにかする」というfixをします。

さて、昨日挙げたサンプルソースについてJavadocをつけて考えてみましょう。まぁそもそも「fooとかbarとかhogeなサンプルコード」なので、具体的な仕様は書きようがないんですが。実践的なコードにするとノイズになってしまうので、定義がビミョーなのはご愛嬌。

FooのバグとなるJavadoc

/**
 * (Fooについての説明)
 */
public class Foo {
  private int bar;

  /**
   * このインスタンスのbarを返す。
   * 
   * @returns このインスタンスのbar
   */
  public String getBar() {
    return String.valueOf(bar);
  }

  /**
   * このインスタンスのbarを設定する。
   * 
   * @param bar このインスタンスのbar
   */
  public void setBar(String bar) {
    this.bar = Integer.parseInt(bar).intValue();
  }
}

/**
 * プログラムのエントリポイントとなるmainメソッドを持つクラス。
 */
public class Main {

  /**
   * プログラムのエントリポイントとなるmainメソッド。
   * 
   * <p>このプログラムを実行すると、標準出力に "hoge" と出力し、終了する。</p>
   * 
   * @param args コマンドライン引数。このプログラムでは無視される。
   */
  public static void main(String[] args) {
    Foo foo = new Foo();
    foo.setBar("hoge");
    System.out.println(foo.getBar());
  }
}

まずFooクラスについて。setBar("hoge") するとNumberFormatException（以下NFE）が飛びます。これが「実際の挙動」。そして「仕様」にはNFEに関する言及は全くない。NFEを飛ばすという記述がない限り、どんな場面でもNFEが飛んだらバグなんです。

この時点でMainクラスを見てみますが。Mainの挙動も仕様と異なります。が、原因はFooの方にあるので、あまり考える必要はない。Fooを直してから考えるべきことです。

従って、FooのJavadocにNFEに関する記述を追加する（仕様バグと判断した場合）か、NFEが飛ばないように何らかの修正をする（実装バグと判断した場合）必要があります。どちらにせよ、setBar()には「引数として受け入れられないもの」が全く定義されていないので、全ての可能性を受け入れるべきです。この仕様のままだったら、barのフィールドをString型にして、ふつーのJavaBeansにするのがいいのでしょうね。なぜIntegerに変換しているのかがよく分からない。（サンプルコードでなく実戦的コードでは、この辺りもJavadocで明らかにしなければならないところである）

MainのバグとなるJavadoc

/**
 * (Fooについての説明)
 */
public class Foo {
  private int bar;

  /**
   * このインスタンスのbarを返す。
   * 
   * @returns このインスタンスのbar
   */
  public String getBar() {
    return String.valueOf(bar);
  }

  /**
   * このインスタンスのbarを設定する。
   * 
   * <p>barにはintの範囲の整数の文字列表現しか設定できない。
   * 一般的に、正規表現 "(+|-)?[0-9]+" で表される文字列の入力を想定している。
   * 厳密にはInteger#parseInt(String)の挙動によるので、参照のこと。</p>
   * 
   * @param bar このインスタンスのbar
   * @throws NumberFormatException 引数barがInteger#parseInt(String)で解釈できない文字列だった場合
   */
  public void setBar(String bar) {
    this.bar = Integer.parseInt(bar).intValue();
  }
}

/**
 * プログラムのエントリポイントとなるmainメソッドを持つクラス。
 */
public class Main {

  /**
   * プログラムのエントリポイントとなるmainメソッド。
   * 
   * <p>このプログラムを実行すると、標準出力に "hoge" と出力し、終了する。</p>
   * 
   * @param args コマンドライン引数。このプログラムでは無視される。
   */
  public static void main(String[] args) {
    Foo foo = new Foo();
    foo.setBar("hoge");
    System.out.println(foo.getBar());
  }
}

これはFooのバグとされるケース。Foo#setBar(String) の仕様と実際の挙動は合致している。"hoge"を与えたらNFEを飛ばすという挙動が仕様から明らかです。

まぁ、別の問題はある。「parse」というアクションに対して「数字の書式が違う(NFE)」という結果を返すは妥当だが、「set」に対して「NFE」というのはいただけない*4。「set」に対しては「引数がおかしい(IllegalArgumentException)」という結果を返すのがベターだ。まぁ余談なので、今回はこれ以上深く触れないことにする。

NFE extends IAEだよ、と教えてもらった。勘違いだｗ これなら問題ない。

そして問題があるのはMainの方で、NFEを飛ばすようなオペレーションをして、実際の挙動がそうなっている。が、Mainの仕様は「標準出力に "hoge"」だそうだ。食い違いがあるのでバグ。

バグなしとなるJavadoc

/**
 * (Fooについての説明)
 */
public class Foo {
  private int bar;

  /**
   * このインスタンスのbarを返す。
   * 
   * @returns このインスタンスのbar
   */
  public String getBar() {
    return String.valueOf(bar);
  }

  /**
   * このインスタンスのbarを設定する。
   * 
   * <p>barにはintの範囲の整数の文字列表現しか設定できない。
   * 一般的に、正規表現 "(+|-)?[0-9]+" で表される文字列の入力を想定している。
   * 厳密にはInteger#parseInt(String)の挙動によるので、参照のこと。</p>
   * 
   * @param bar このインスタンスのbar
   * @throws NumberFormatException 引数barがInteger#parseInt(String)で解釈できない文字列だった場合
   */
  public void setBar(String bar) {
    this.bar = Integer.parseInt(bar).intValue();
  }
}

/**
 * プログラムのエントリポイントとなるmainメソッドを持つクラス。
 */
public class Main {

  /**
   * プログラムのエントリポイントとなるmainメソッド。
   * 
   * <p>このプログラムを実行すると、NumberFormatExceptionが発生した後、終了する。
   * Fooの仕様を考慮しないと、直感で「標準出力に "hoge" が出力される」という挙動になりそうだが、
   * 実際はNumberFormatExceptionが発生してしまう状況を表現している。</p>
   * 
   * @param args コマンドライン引数。このプログラムでは無視される。
   */
  public static void main(String[] args) {
    Foo foo = new Foo();
    foo.setBar("hoge");
    System.out.println(foo.getBar());
  }
}

これは、FooもMainも、仕様と実際の挙動が完全に合致している。Mainに色々書いてあるが、System.out.println(...)という「絶対に実行されないけどわざわざ何か書いてある」ことを正当化する仕様になっているのが分かると思う。ただ単に「NFEが飛ぶ」という仕様であれば、なぜ「throw new NumberFormatException()」と書かないのだ、という議論になるかもしれないので。

上記のJavadocでもまだ甘いところがあるかもしれません。が、ここでは「FooやMainの仕様を定義すること」ではなく「こんなことを仕様に盛り込むべきなんだよ」という方向付けが趣旨なので。この辺にしておきますｗ（逃げ）

Javadocに書くべきこと

というわけで、コード部は全く一緒です。上記のそれぞれの違いはJavadocだけ。Javadocによって、何がバグなのかが決まる様子が分かるかと。

「Javadocに書かれていない挙動をしたらバグ」です。そしてJavadocには「この文章を読むだけで、そのクラスやメソッドの実装を漏れ無く復元できる」だけの情報を記述すべきです。

例えばよくある、setterで「状態が変化したイベント」を飛ばしたりする例。

/**
 * (ç•¥)
 */
public class Hoge {
  private Collection<PropertyChangeListener> listeners;
  private String fuga;

  /**
   * Fugaを設定する。
   *
   * @param fuga 新たに設定するFuga
   */
  public void setFuga(String fuga) {
    String old = this.fuga;
    this.fuga = fuga;
    PropertyChangeEvent e = new PropertyChangeEvent(this, "fuga", old, fuga);
    for (PropertyChangeListener l : listeners) {
      l.propertyChange(e);
    }
  }

  /**
   * このインスタンスのフィールド変更を検知するリスナを追加する。
   *
   * @param listener 新たに追加するリスナ
   */
  public void addListener(PropertyChangeListener listener) {
    listeners.add(listener);
  }

  // その他色々。listenersを初期化したりしなきゃいかんですが省略。
}

これが、事故*5でこうなってしまった場合…。

/**
 * (ç•¥)
 */
public class Hoge {

  /**
   * Hogeを設定する。
   *
   * @param hoge 新たに設定するHoge
   */
  public void setHoge(String hoge) {
    // 消えちまったー
  }

  /**
   * このインスタンスのフィールド変更を検知するリスナを追加する。
   *
   * @param listener 新たに追加するリスナ
   */
  public void addListener(PropertyChangeListener listener) {
    // 消えちまったー
  }

  // 省略。
}

このドキュメントから元通りの実装復活できますか？ まぁこの程度なら出来ないこともないかもしれないけど。実戦的なコードならまず無理。

/**
 * Fugaを設定する。
 * 
 * <p>Hogeを設定した後、このクラスに登録済みの全てのPropertyChangeListener
 * に対してfugaフィールドが変更されたことを表すPropertyChangeEventを送出する。</p>
 * 
 * @param fuga 新たに設定するFuga
 */

とか書かれていれば、なんとか元に戻せそうです。まぁこのコードも適当に書いた仕様なので、甘いかもしれませんがｗ（再び逃げ）

さて、ちなみにsetFuga()のJavadocを↑のように修正しても、まだバグが残ってます。是非見つけてみてください。（コンパイラに掛けず、テキストエディタでコード書いてるので、俺も意図していないバグ入ってるかもしんないけどｗｗ）

まとめ

バグの定義をするためには、Javadoc（仕様）が必要です。コーディングルールとしては、まず「仕様と挙動の食い違いがをバグとする」と明示する。そうすると自動的に「Javadocに何を書くべきか」が決まる。

コーディングルールに「事前条件を書きなさい」とか「RuntimeException系でも@throwsに書きなさい」とか、細かい粒度を列挙して網羅しようとせず、まず大綱として上記を定義すると良いと思う。ただ、これだけでは「ノリが分からない人」に伝わらない可能性があるので「具体例として、事前条件が書かれてなかったら、事前条件なしと判断する」とか「RTE系は@throwsに書いてなければ、投げないと判断する」というような例示をして補足する。

*1:この挙動はFooのバグでしょうか？Mainのバグでしょうか？...

*2:前述の通り、仕様が不明確だからわかんねー的な反応が多かったが、その通り。前回でJavadocを書いてしまうと、1〜4を列挙できず、その場で1つに決まってしまうので敢えて書かなかった。

*3:どちらもfixのためにはソースファイルを直すことになります。

*4:会話が成立していない

*5:都合のいい事故だ…ｗ