引数チェックと assert
後輩のコードレビューをしていて、引数チェックはどういう風にするのがいいのか、みたいな話になった。
「どういう風に」といってもいろいろあるが、今日の話題は、以下の2派閥のどっちがいいのかということ:
- 引数チェックは常にするよ派
-
void methodA(final ClassX parameterX) { if (isValid(parameterX)) { throw new IllegalArgumentException("引数が不正"); } methodB(parameterX); } void methodB(final ClassX parameterX) { if (isValid(parameterX)) { throw new IllegalArgumentException("引数が不正"); } doSomethingWith(parameterX); }
- 引数チェックなんかしないよ派
-
void methodA(final ClassX parameterX) { methodB(parameterX); } void methodA(final ClassX parameterX) { doSomethingWith(parameterX); }
それぞれのメリットとデメリットは、こんな感じ:
メリット | デメリット | |
---|---|---|
「するよ」派 |
|
|
「しないよ」派 |
|
結論としては、「しないよ」派が勝利。
勝因は、“「するよ」派の主張するメリットは assert
で得られる”というありきたりなものだったが、以下の主張・考察は少し新鮮だった:
- 「
assert
は-enableassertions
でないと無効」だから、テストを充実させないと無意味。 - テストで見つけられなかったロジックエラーが実運用で発生したら、テストが悪い。
- 引数チェックにかける情熱は、条件網羅・分岐網羅なテストの実現に傾けた方がいい。
- 不正な引数をもらった場合、大概は、そのうち何かの例外が発生する。スタックトレースとかも見れるし、わざわざ
IllegalArgumentException
を投げることの実効性は疑問。 - 例外メッセージをたくさん書かなければならなくなると、結局、1つ1つは手抜きになる。そういう診断メッセージの方がよっぽど分かりにくい。
ただ、「セキュリティホールになるんじゃないか?」には明確な解答は出せず、「不正な引数がもたらす結果が深刻な場合にはチェックしておこう」みたいにお茶を濁した。
はい、トートロジーなのは分かっています。