Javaのassertは使用が推奨されない？

Question

assert i == 5 : "i must be 5";

javaの組み込みのassert機能って使い方によっては便利そうな気がするのですが、現場で使われているのを私はほとんど見たことがありません。

これは製品コードに一種のデバックコードが紛れるのが嫌わたり、単体テストが普及して価値を失ったなど何か理由があるのでしょうか？

また、開発現場で実際に使っている(いた)方がいらっしゃれば、使ってよかった・悪かった点などご教授いただけないでしょうか？

Answer 1

アサーションについて、私の考えが既出の意見とは異なるので参戦。

アサーションは、サブルーチンなど、かなり下位の（内部的なモジュールの）インタフェース仕様をコメントのかわりにいれるものと考えています。
コメントで

// 引数Xは必須で null が渡されてくることは考慮していない

と書くかわりに

assert(X!=null)

と書くわけです。コメントと比較して良いところは、結合テスト中にインタフェースミスが
はっきりと分かるところです。 NullPointerException だと、「サブルーチン作成側の考慮漏れによるバグ」なのか「担当者間のインタフェースミス」なのか言い争いになるところを assert を書くことによって「担当者間のインタフェースミス」であることが確定するところにメリットがあります。

一方で assert を書くくらいならそのチェックを正式コードとして入れてはどうかという説もあるかと思います。しかし、内部モジュールのインタフェースミスと Exception という正規の異常系処理ロジックを混在させることは、可読性をさげるとともに、デバッグを難しくします。

public abc(String x, String y) {
  if (x == null) {
    throw Excpetion("予想外のエラーが発生しました（関数 abc の第一引数に null が渡されました）");
  }
  if (y == null) {
    throw Excpetion("予想外のエラーが発生しました（関数 abc の第二引数に null が渡されました）");
  }

というようなコーディングを多く見かけるわけですが、これだけだと、この Exception が発生したときにエンドユーザが入力をミスしたのか、システム障害でI/O に失敗したのか、インタフェースミスのバグなのかがコードから判定がつかないわけです。これを assert で書いておけば、インタフェースミスのバグであることが確定でき、可読性を向上させるとともに、トラブルシューティングを簡単にすることが可能です。

Comments

[baseballyama]

アプリ構造によって色んな状況があると思いますので個人の意見にすぎませんが記載させて下さい。

「仕様通り」に「NullPointerException」を発生させる構造は良くないと思っています。
なぜならば、意図しない「NullPointerException」と仕様通りの「NullPointerException」という複数の意味を1つのExceptionに持たせてしまっているからです。
これでは毎回Exceptionが発生するたびに仕様を確認しなければならず、間違った障害対応を実施するリスクを抱えてしまうと思います。これでは「有識者」頼みの属人的プロジェクトになってしまいます。
1つのExceptionは1つの意味と紐づいているべきです。

また、
>// 引数Xは必須で null が渡されてくることは考慮していない
というコメントを書かなければならないこと自体、アプリ構造に問題ありと考えています。
このコメントが出てくるということは、「入力値の正当性担保」という責務を誰も負っていない状態ではないでしょうか。
「入力値の正当性担保」がされた状態でこのクラスに渡ってくる構造であれば、Nullチェックは必要なく、これはつまり、
「NullPointerException」が発生した　＝　どこかの処理が間違っている、
と考えられるのではないでしょうか。

仕様としてNullがあり得るのであれば、IF文などでNullかどうか判定し、Nullだった際はそのままReturnなり、Null時のロジックを記載すれば良いのだと思います。

[mit0223]

少し誤解を招いたようなので、修正しました。私の主張は NullPointerException とコメントよりも assert のほうがいいよねという主張ですが、 baseballyama さんのコメントは賛同コメントでしょうか？

> 仕様としてNullがあり得るのであれば、IF文などでNullかどうか判定し、Nullだった際はそのままReturnなり、Null時のロジックを記載

インタフェース仕様としてそのように設計できればいいのですが、内部モジュールの場合、そのような設計が逆に冗長になり、性能や品質に悪影響を与える場合もあります。assert は適材適所だと思います。

[mit0223]

ちょっと読み飛ばしてましたので追記です。
basballyama さんは「仕様としてNullがあり得るのであれば」とコメント頂きましたが、私が assert を使うべきと考えているのは「仕様としてNullがあり得ない」場合です。

[baseballyama]

回答の修正ありがとうございます。理解しました。
ただ、「サブルーチン作成側の考慮漏れによるバグ」は単体テストで潰れているべきではないでしょうか。
潰れていないということは単体テストのプロセスに問題ありと思います。

>baseballyama さんのコメントは賛同コメントでしょうか？
個人的には反対です。assertの致命的にいけないところは処理がそこで終わってしまうことです。
また、テストのためのコードを本番資産に記載することは望ましくないという考えです。
他の手段で代替できるのであれば他の手段で代替するべきと思っています。

Exceptionが発生したらフレームワークによって適切にハンドルされて、「何が原因でどんなエラーが発生して、ユーザーは次に何をすべきなのか」を画面などに出力すべきだと思っています。
※ただ、内部モジュールの場合にはこのケースはほぼないと思いますので、Exceptionが発生　＝　どこかのロジックが誤っている、と考えられると思います。

>内部モジュールの場合、そのような設計が逆に冗長になり、性能や品質に悪影響を与える場合もあります。
追加コメントで頂きましたが、仕様としてNullがあり得ないのであれば、「担当者間のインタフェースミス」で確定しませんか？冒頭にも記載した通り、「サブルーチン作成側の考慮漏れによるバグ」は単体テストで潰れているべきだからです。単体テストの漏れを恐れて本来必要のないassertを記載することは屋根の上に屋根を被せるようなものです。

>assert は適材適所だと思います
これはその通りだと思います。assertの特性を生かせる場所はあると思います。

色々書いてしまってすみません。私も知識に富んでいる訳ではないので、
間違いなどありましたらご教示頂きたく思っています。何卒よろしくお願い致します。

[mit0223]

baseballyama さん、貴重なコメントを頂きありがとうございます。コメントを参考にさらに回答を編集してみました。

[baseballyama]

Exceptionは各クラスではなくスーパークラスで発生させればよい(そしたら業務ロジックスーパークラスに1箇所書けば済む)と思います。
加えて、引数のチェックなどという各クラスが一様に記載するようなコードはDRY法則に従ってどこか1箇所に記載されるべきと思います。

話は少しそれますが、今回のNullPointerExceptionのケースであればわざわざExceptionを記載しなくても勝手にExceptionするので、単純にこのクラスではthrowsしてあげてフレームワークなどでハンドルすればよく、わざわざ個別クラスに記載すべき内容ではないと思います。

障害解析においても、Traceレベルで引数などを出力させておけば解析は容易と思います。(このログ出力も無論、スーパークラスに記載するかAOPを使用して実装する)

[ken_murayama]

皆様ご回答ありがとうございました。

どの意見も大変参考となりましたが、mit0223さんとbaseballyamaさんの議論が大変興味深かったので起点となりましたmit0223さんのコメントをベストアンサーとさせていただきます。

個人的にはprivateなルーチンの一部で引数の範囲チェックなどにassertを使用してみて、レビュー時にチームメンバーと議論してみようと思います。何か有益な知見が得られたらこちらのコメントに追記いたします。

[baseballyama]

今更ですがwebで調べてみたらこんなページがありましたのでご参考までに共有します。
https://www.jpcert.or.jp/java-rules/met01-j.html

[ken_murayama]

このサイト、サンプルコードも充実してるし、各ページの記述が統一されていていてassert以外の記事もとても参考になりますね。ご共有ありがとうございます！

Answer 2

JavaだけでなくC/C++やC#にもassertがありますが、確かにあまり使っているのを見たことが無いような気がします。

assertは使い所がかなり限定されたものだと思っています。assertは例外チェックでは無くバグチェック以外にしか使えないからです。

###引数チェック

まず、引数チェックにassertが使えるかです。publicメソッドの引数にassertを使うことは間違っています。もう、初っぱなから全否定です。ただ、privateメソッドに使うことは悪いことではありません。

public class Main {
    private void puts(String str) {
        assert str != null;
        System.out.println(str.toUpperCase());
    }
    public void run() {
        String str = "Hello, world!";
        puts(str);
    }
    public static void main(String[] args) {
        Main main = new Main();
        main.run();
    }
}

puts()はprivateなので同じクラス内から呼び出せません。そしてnullは渡してはいけないとしましょう(ヌルポになっちゃいますので)。nullを渡さないというのはクラス内の他のメソッド責任です。もし、nullが渡された場合は、バグがあるのはその他のメソッドと言うことになります。

この場合はassertでnullチェックをすることは問題ないと思います。上のコードは単純すぎるのでassertでなくてもわかるかも知れませんが、整数の範囲とか、特定の文字列のみ許しているとか、そういった場合はassertを使った方が何が問題なのかがわかりやすいでしょう。

そして重要なのはUnitTestではどんな引数でそのメソッドを呼び出すのかがチェックできないと言うことです。UintTestは全てのパターンをテストできるわけでは無いため、一部のパターンだけで想定通りの値を必ず渡すとすることはできません。また、テスト失敗時に、何で失敗したのか、何が想定外だったのかも、assertを用いるとわかりやすくなります。

###処理後のチェック

大昔に(訳ありで)書いたJavaのコードからの抜粋を紹介します。(私の本職はJavaプログラマーじゃ無いので、細かいところは大目に見てください。)

	protected static int[] getLineScore(List<Integer> lineList) {
		int[] score;
		score = Com.lineScoreMap.get(lineList);
		if (score == null) {
			score = calcLineScore(lineList);
			// 新たに作成してからMapに配置しないといけない！
			// 計算結果は常胃同じはず。
			// 変更しないので同期もいらないはず。
			Com.lineScoreMap.put(new ArrayList<Integer>(lineList), score);
		}

		assert score.length == 3 : "スコアのサイズが3じゃない、score.length = " + "depth:"
				+ score.length;

		return score;
	}

コメントに誤字が混ざってますが、そこはご愛敬で。caleLineScore()は必ず長さ3のintの配列を返すようになっています。もちろん別途UnitTestは存在しますが、本当に常に長さ3のintの配列を返すかどうかはわかりません。また、キャッシュしているCom.lineScoreMapに変なものが混ざっている可能性もあります。そもそも同期いらないとか書いているけど、これ本当なのかな…。

ぶっちゃけ、怪しすぎるので戻ってきた値や前の処理をassertでチェックしています。もし、おかしい値が返ってきていたら、その後の処理がうまく動かないからです。

もし、assertがなかったら、その後の処理の深いところでエラーになるかも知れません。そこから、どこで間違っていたかを辿ることはかなり大変です。ですが、ここでassert失敗になれば、その前のcaleLineScore()あたりがおかしいとあたりを付けられます。バグの原因もすぐに見つけられるでしょう。

###デバッグの時だけエラー

assertは実行時にオプションを付けないと有効になりません。本来、想定とは違う場合は、なんらかのバグが発生しているため、停止してエラーになった方がデバッグがしやすくなります。しかし、本番環境ではある程度の想定外は無視してなるべく安定して動いて欲しいと思います。また昔のコードの抜粋です。

	public static boolean isFrozenBoardList(List<List<Integer>> boardList) {
		if (boardList.get(Com.LINES).get(5) == 0) {
			return false;
		} else {
			assert boardList.get(Com.LINES).get(5) == 1 : "固定フラグが未知数です。"
					+ boardList.get(Com.LINES).get(5);
			return true;
		}
	}

boardList.get(Com.LINES).get(5)は必ず0か1です。バグが無ければですが。ですが、本番環境では、もし、想定外の値だった場合でもそのまま進みたいと考えています。逆にデバッグ時はなるべくすぐにエラーになって止まって欲しいと考えています。二つの動作が行えるようにassertを使っていると言うことです。

---

私の思いつくのはこれぐらいです。上の抜粋したコードは別途UnitTestも存在し、そちらで単体テストも行っています。しかし、全てのパターンも網羅することは不可能なので、結合テストの時にassertを有効にして、想定外の動作がしていないかをチェックするようにしました。

assertは本番では無効にして使う物だと思っています。単体テストの時に想定外の値になっているかのチェックをわかりやすくする、結合テストの時に単体テストでは網羅できなかったパターンがやってきて、想定外の値になっていないかチェックする。そういったことぐらいにしか使えないと思います。ただ、assertは任意のメッセージを書くことができますので、うまく使えば、デバッグがしやすくなる、バグの原因追求が早くなると思います。

まぁ、私はJavaプログラマーでは無いので、Javaはほとんど書かないから、実際の現場がどうしているのかはちょっとわからないのですけど。

Comments

[ken_murayama]

大変丁寧にご回答いただきありがとうございます。
サンプルコードも大変ありがたいです。

> assertは本番では無効にして使う物だと思っています。単体テストの時に想定外の値になっているかのチェックをわかりやすくする、結合テストの時に単体テストでは網羅できなかったパターンがやってきて、想定外の値になっていないかチェックする。そういったことぐらいにしか使えないと思います。ただ、assertは任意のメッセージを書くことができますので、うまく使えば、デバッグがしやすくなる、バグの原因追求が早くなると思います。

もし使うとしたら確かにこの辺になりそうですね。

Answer 3

javaの組み込みのassert機能についてですが、assert結果がFasleの場合にプログラムが終了するのはご存知かと思います。
通常のプログラムにおいては、assert結果がFalseの場合、Exceptionを発生させて上で、
エラー処理クラスでExceptionをキャッチし、エラー内容をユーザー画面に返却する処理が実装されます。
なぜならばただシステムエラーが返却されてもユーザーが次に何をしたらいいかわからないからです。

よって、assertではなくif文などでパラメータ(＝引数)チェックを実施し、不正な値が入っている場合にExceptionなどを発生させる処理を実装することが一般的と思います。
※複雑な業務アプリケーションになると、業務ロジックの前に全てのパラメータのチェックを実施する処理を用意し、不正な値が入っている場合には業務ロジックに入る前に画面に折り返してしまう場合もあります。そのような場合は業務ロジッククラスは正しい値が入ってくる前提で処理を実装しても良い場合があります。

以上の理由により、javaの組み込みのassert機能は使われないと考えています。

Answer 4

以下が参考になるかと。
Javaのアサーションと単体テストについて

ただ、両方しっかり書いているコードにはなかなかお目にかからないというのが個人的な感覚です。
(特にアサーションがないほうが多い印象)

Comments

[ken_murayama]

私は恥ずかしながらassert機能について見たことがなく、昨日初めて知ったところです。
リンクの質問と回答にも目を通しました。ご共有ありがとうございました。