堅牢なコーディングルールを策定する方法(2)
いやー、なんか怖い人(笑)が見てるようだ。突っ込み激しそうぜよw
さて、前回言っていた「判断ロジック」についての答えは各自考えてみただろうか? 各方面の反応を見ると「1〜4どれでしょうか*1」という問いにすり替わっちゃってる気がするけど、テーマは「1〜4を決めるロジックを考えてください」なんだ。書き方悪かったもしれんw まぁつまり、昨日のエントリの時点では1〜4どれでもないのですよ。判断ロジックがないので。
というわけで本題。アプリケーションレベルでのバグとは「実際の挙動が仕様と違うとき」ですね。挙動があるべき姿(=仕様)と違う時にそいつバグとするのが妥当です。そしてそれはコードレベルでも同じ基準で良いんじゃないでしょうか。
じゃあ、仕様とは何か。Javadocコメントですよ。Javadocがないとバグの定義さえもできないんです。だから口酸っぱくJavadoc書けとw*2
まぁつまり「Javadocに書かれていない挙動をしたらバグ」です。というわけでJavadocを明確に記述します。
ちなみに、今回は例示しなかったが、基底クラスやインターフェイスの記述も重要です。例えば、基底クラスのhoge()メソッドに@throws SomeRuntimeExceptionが宣言されていない限り、サブクラスのhoge()でSomeRuntimeExceptionを投げてはならんです。そのインスタンスをインターフェイス越しに見た時に見えるJavadocは、インターフェイスのものなので。そこに記述されていない「SomeRuntimeExceptionを投げる」という挙動をすることになってしまいます。
まぁそんなJavadocを記述すると、バグかが明らかになりますね。で、バグには2種類考えられます*3。
前述の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だよ、と教えてもらった。勘違いだw これなら問題ない。
そして問題があるのは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の仕様を定義すること」ではなく「こんなことを仕様に盛り込むべきなんだよ」という方向付けが趣旨なので。この辺にしておきますw(逃げ)
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 */
とか書かれていれば、なんとか元に戻せそうです。まぁこのコードも適当に書いた仕様なので、甘いかもしれませんがw(再び逃げ)
さて、ちなみにsetFuga()のJavadocを↑のように修正しても、まだバグが残ってます。是非見つけてみてください。(コンパイラに掛けず、テキストエディタでコード書いてるので、俺も意図していないバグ入ってるかもしんないけどww)
まとめ
バグの定義をするためには、Javadoc(仕様)が必要です。コーディングルールとしては、まず「仕様と挙動の食い違いがをバグとする」と明示する。そうすると自動的に「Javadocに何を書くべきか」が決まる。
コーディングルールに「事前条件を書きなさい」とか「RuntimeException系でも@throwsに書きなさい」とか、細かい粒度を列挙して網羅しようとせず、まず大綱として上記を定義すると良いと思う。ただ、これだけでは「ノリが分からない人」に伝わらない可能性があるので「具体例として、事前条件が書かれてなかったら、事前条件なしと判断する」とか「RTE系は@throwsに書いてなければ、投げないと判断する」というような例示をして補足する。