トップ «前の日記(2005-05-18) 最新 次の日記(2005-05-20)» 編集

日々の破片

著作一覧

2005-05-19

_ 誤った例外の使い方

public void foo(String a, String b, String c) {
    if (a == null || b == null || c == null) {
        throw new IllegalArgumentExcpetion();
    }
    int lena = a.length();
    int lenb = b.length();
    int lenc = c.length();
    ...
}
馬鹿ですか?
本日のツッコミ(全14件) [ツッコミを入れる]
_ ten (2005-05-20 08:07)

いやいや沢山ありますよ。こういうコード……<br>大抵、例外機構のない言語をやってた人みたいですけど。

_ 初心者 (2005-05-20 10:28)

念のため確認なんですが、throw new IllegalArgumentException("例外の内容" + arg);とかだったら良いんですよね?

_ arton (2005-05-20 14:11)

そうとも言えないです。>初心者さん<br>上の例だと、NullPointerExceptionがスローされた行からa,b,cの最初のnullがわかるからnullチェックそのものにあまり意味がないからです。<br>このメソッドを呼び出しているのがユーザー入力を扱うプログラムであれば、そのプログラムが正しくnull(無入力かな)はだめとか出力してこのメソッドの呼びだしをブロックすればいいわけだし(その意味では、呼び出し側のバグだからNullPointerExceptionで良い)、そうでなくてサーバーサイドプログラムであればどちらにしても処理は継続できないので、これまたNullPointerExceptionで良い。<br>ただし、呼び出しプログラムのバグを修正する場合には例外メッセージとスタックトレースがヒントになるから、仮にfooの実装が<br>int totallen = a.length() + b.length() + c.length();<br>だとどれがnullか不明で調査しにくいかも知れないから、例外にメッセージとして<br>new IllegalArugmentException("a=" + a + ",b=" + b + ",c=" + c);<br>とすれば親切な設計で、それならそれで良いのです。でも最初にnullを検証した変数を1つだけnullだと指摘するのなら、NullPointerExceptionにしてしまったほうがよけいなコードがなくてすっきりするのでより望ましいです。<br>結論としては、僕は次のようにして先頭の3行を削除するのが良いと思います。<br>/**<br> * ....<br> * @throws NullPointerException a,b,cのいずれかがnull。<br> */<br>public void foo(String a, String b, String c) {<br> int len = a.length();<br> ...<br>}<br>tanさんが書かれているように、例外がどういうものでどのような効能があるのか意味がわからないと、自分でIllegalArgumentExceptionをスローするほうが、勝手にNullPointerExceptionがスローされるより良いような気分になりやすいので無駄なスローを記述しやすいのですが、呼び出し側からすればダメなものはダメ、バグはバグで同じ意味しかありません。ならば、無駄なコードは記述しないほうが良いでしょう。<br>#この例は、メッセージさえ入っていないから論外。

_ 通りすがり (2005-05-20 14:42)

私も初心者なので「何が悪いんだろう?」としばらく考え込んだわけですが、「どーせIllegalArgumentExceptionも非チェック例外なんだからぬるぽ吐くに任せりゃええやん、何が悪かったのかも分かりやすいし」という結論に達しました。

_ arton (2005-05-20 14:49)

あ、名前書き間違えてた。ごめんなさい。tanじゃなくてtenさんでした。

_ arton (2005-05-20 15:00)

>通りすがりさん<br>そうです。「何が悪かったのかも分かりやすい」が重要です。だって、例外食らうのは呼び出し側だから。<br>例にしたnullチェックは何がIllegalかわからないし(何しろメッセージが無い)、しかもnullかどうかはどっちにしても後続の処理でJVMによって検証されるわけで重複処理だし、単にポーズとして行をたくさん書きましたと言ってるだけなので最低だということです。<br>#たとえば、nullの書き込みが致命的なテーブルにそのまま、pstatement.setString(1, a); とかしてしまう可能性がありえるメソッドならnullチェックはしなければならないわけだけど(チェックしなければ例外にならずにデータベースを破壊しながら突っ走るわけだから)、この例は明示的にa.length()とか呼んでるわけでそんな心配は無用。ということから、元のコードを書いたやつはString#lengthの処理内容とかそれによってNullPointerExceptionになるってことすらわからずに書き殴ってるのか? という疑いも出てくるわけですね。

_ 初心者 (2005-05-20 15:03)

なるほどー。<br>よくよく考えれば当たり前のことなのに、馬鹿丸出しですね。 > 自分<br>目からぼろぼろ鱗が落ちた気分です。勉強になりました、ありがとうございます!

_ arton (2005-05-21 12:55)

馬鹿丸出しってことはないですよ。メッセージをちゃんと入れることで障害原因の発見が早まる可能性もありますから。丸出しなのは、例のように、他の機構に任せることもしなければ、かと言って必要な情報も出さないプログラムです。<br>jarでバイナリ配布しかしなければ、スタックトレースの行番号はほとんど意味が無い(いろいろ実験して変化を追うこととかは可能かも知れない)から、NullPointerExceptionおまかせではなく親切にメッセージを入れて自分でスローしたほうがよりベターかも知れません。結局、そのプログラムが実行されるコンテキストにも依存してきます。<br>極端なことを言えば、fooを呼んでいるのがfooを作った人と同じなら、メッセージが無くても引数がnullだとわかるし(と書いたが、だったらNullPointerExceptionお任せにすべきだな)。<br>いずれにしても、スローするなら状況がわかる(期待値と与えられた値の併記とか)メッセージをつけること、それが基本だと思います。

_ unibon (2005-05-21 16:58)

javadoc の @param に「null 不可」と書く代わり、と見ることもできるのではないでしょうか。なお、些細なことですが、私だったら、<br>public void foo(String a, String b, String c) {<br> if (a == null) {<br> throw new IllegalArgumentExcpetion();<br> }<br> if (b == null) {<br> throw new IllegalArgumentExcpetion();<br> }<br> if (c == null) {<br> throw new IllegalArgumentExcpetion();<br> }<br> int lena = a.length();<br> int lenb = b.length();<br> int lenc = c.length();<br> ...<br>}<br>のように分けて書きます。<br>また、上述のように分ける、分けない、に関わらず、たとえば、foo のロジックが a の値によって b をまったく参照しないことががごくまれにあるような場合、メソッドの入り口で null かどうかを検査しておけば、呼び出し元がもし引数 b を null にして foo を呼び出していれば、呼び出し元がおかしいことはすぐに分かります。これは利点になります。public なメソッドならば、やる意義は高いと思います。<br>null 以外には、ClassCastException が出るよりも前に捕まえるために instanceof でチェックすべきか、という話も出てくるかもしれませんね。

_ arton (2005-05-21 19:29)

>unibonさん<br>あまり感心しません。@paramにnull不可と書く代わりにはなりませんよね。@paramにnull不可と書いてあるからこそ、@thorwsにNullPointerExceptionと書けるのではないでしょうか。null可なのに(不可と明記されていない場合は、呼ぶ側は自分に都合が良いように解釈すると想定すべきなので)NullPointerExceptionがスローされたらそれはfooの実装がバグだと思います。<br>分けるとしてもツッコミ欄で細かく書けないからだとは思いますが、9行も使うものではないでしょう。もし、そのチェックをしたいのであれば、<br>private String filterIfNull(String s, String name) {<br> if (s == null) {<br> throw new IllegalArgumentException(name + "がnull");<br> }<br> return s;<br>}<br>...<br>public void foo(...) {<br> int lena = filterIfNull(a, "a").length();<br> ...<br> // 既にフィルタリングしてあるからaを自由に使える。<br> if (a.indexOf('a') ....) {<br> ...<br>}<br>という感じでしょう。<br>メソッドの入り口でチェックすべきかどうかは処理時間に依存するかも知れませんね。

_ ten (2005-05-22 13:52)

あら? 何かコメントが沢山だ……<br>> あ、名前書き間違えてた。ごめんなさい。tanじゃなくてtenさんでした。<br>いえ、お気になさらずに。<br>仰る様に、例外機構の本質を理解してないと、意味の無いコードを書いてしまうことが多いのでしょうね。<br>今回の元記事の例は、実行時例外が発生することが自明の状況に対して、わざわざチェックして throw する様なコードを書くことが冗長であり、有害ですらあるということだと理解しています。<br>システムが例外を生成しないが、アプリケーションとしては問題がある場合には、自ら例外を生成してやる必要があるのは当然ですが。<br>例外機構の本質という表現をしましたが、私自身は、例外機構そのものについて、<br>(1) 例外となる事象の発生による割り込み<br>(2) 割り込みを捕捉することによる大域脱出 (とその後始末)<br>と捉えています。<br>そう言う意味では、例外機構は「プログラムの流れを乱す何らかの事象が発生したときに、できるだけスムーズに、正しくプログラミングされた流れに引き戻す」ためにあると言えるでしょうか。<br>ただ、アプリケーションの動作に異常が検出されたときに必要なことを、例外機構が全て肩代わりしてくれる訳ではありません。<br>アプリケーションにとって、異常が検出されたときに最も重要なことは、その異常の原因を除去して正常な動作を行える様に回復することです。それが「プログラミングされた流れに引き戻す」ことによって自動的に行える場合は良いのですが、そうでないときには、利用者や実装者による原因除去の作業が必要になり、そのためには、できるだけ原因の特定が速やかに行える様な情報を提供することが望ましいと言えます。<br>java に限って言及すると、最近の Java の実装でこそ、例外オブジェクトから情報を得る手段が多くなった様ですが、J2SE 1.3 位までの実装ではトリッキーな手段を用いないと詳細の情報を得ることができず、アプリケーションによる詳細な異常メッセージの出力のために、余計なコードを書かなければならない局面も多かったと認識しています。<br>異常時にはどんなときでも終了してしまえる様なアプリケーションなら問題が少ないですし、そもそもアプリケーションの性格によって、どの様に振る舞うべきかが異なりますので、どこまでやるべきかを一般的に語ることはできないのかもしれませんが。

_ unibon (2005-05-22 14:26)

@param の件ですが、ちょっとチャチャになってしまいすみませんが、たとえば String クラスの concat メソッドは @param に null についてなにも書いてありませんが、null を渡して呼べば NullPointerException が起きます。<br>また、行数が多い(9行)の件ですが、複雑にならないのならば、ただたんに行が長くなることを避ける必要性はあまり感じません。<br>また、これは好みも入ってきますが、例外はたんに throw new NullPointerException(); だけでいいんじゃないだろうかと思います。エラーの理由などのメッセージ文字列はあれば親切だけど、なくてもコードを見て分かるのなら、メッセージ文字列は付けなくてもいいように思います。9行にするのも、そういう意図があります(行番号で a か b か c のいずれなのかが特定できる)。<br>あと、nullチェックのタイミングについてですが、メソッドの入り口でのチェックということなのか、それともコードの途中の任意の場所なのか、でチェックの意味合いが違ってくると思います。私は前者なら必要性はそれなりにあると思いますが、後者なら不要(length メソッドを呼べばどうせ NullPointerException が起きるのでわざわざチェックする必要はない)と思います。<br>ともあれ、みなさまのご意見を拝見していると、いろいろと見えてなかったことが見えてきて参考になります。

_ arton (2005-05-22 23:03)

上のほう(12:55)に、コンテキストに依存していると書いているのを読んだ上で@paramがどうしたとか書いていたわけではなかったのですね。それならちゃちゃということで良いのではないでしょうか。ただ、あまりお手本にはならない記述をJavasoftのソースだからといって例にするのはそれほど良い方法ではないと思います。また、コードの長さはそれ自体が複雑さに通じます。子供の本(平仮名が多くて教育漢字しかつかっていない)100行と、普通の本の2行なら普通の本2行のほうが単純でしょう。量も複雑さを増すための重要なファクタです。覚えておいたほうが良いでしょう。後は12:55の書き込みの言い換えですね。

_ arton (2005-05-22 23:08)

単純って言葉はちょっと微妙だな。明晰さといったほうが良さそうだ。短いものは明晰である。


2003|06|07|08|09|10|11|12|
2004|01|02|03|04|05|06|07|08|09|10|11|12|
2005|01|02|03|04|05|06|07|08|09|10|11|12|
2006|01|02|03|04|05|06|07|08|09|10|11|12|
2007|01|02|03|04|05|06|07|08|09|10|11|12|
2008|01|02|03|04|05|06|07|08|09|10|11|12|
2009|01|02|03|04|05|06|07|08|09|10|11|12|
2010|01|02|03|04|05|06|07|08|09|10|11|12|
2011|01|02|03|04|05|06|07|08|09|10|11|12|
2012|01|02|03|04|05|06|07|08|09|10|11|12|
2013|01|02|03|04|05|06|07|08|09|10|11|12|
2014|01|02|03|04|05|06|07|08|09|10|11|12|
2015|01|02|03|04|05|06|07|08|09|10|11|12|
2016|01|02|03|04|05|06|07|08|09|10|11|12|
2017|01|02|03|04|05|06|07|08|09|10|11|12|
2018|01|02|03|04|05|06|07|08|09|10|11|12|
2019|01|02|03|04|05|06|07|08|09|10|11|12|
2020|01|02|03|04|05|06|07|08|09|10|11|12|
2021|01|02|03|04|05|06|07|08|09|10|11|12|
2022|01|02|03|04|05|06|07|08|09|10|11|12|
2023|01|02|03|04|05|06|07|08|09|10|11|12|
2024|01|02|03|04|05|06|07|08|09|10|11|

ジェズイットを見習え