Edited at

もういい加減「nullチェックをしたら安全」とかわけのわからないことを言うのはやめよう

More than 1 year has passed since last update.

(僕の主観で)クソな記事をたまたま読んでしまってもにょったので愚痴。


Safe Harbor Statement

以下の事項は個人の見解であり所属する組織の公式見解ではありませんし、明日になったら違うことを思っているかもしれませんがそういう類のものです。


某所で危険と言われていたコード

public void hogeMethod(Hoge hoge) {

//hogeを使った処理
hoge.doSomething();
return;
}

危険な理由: hogeがnullだったらNullPointerExceptionが発生する


某所で安全と言われていたコード

public void hogeMethod(Hoge hoge) {

if(hoge == null) {
return;
}
//hogeを使った処理
hoge.doSomething();
return;
}

安全な理由: NullPointerExceptionが発生しない


僕がいいたかったこと

それ、NPEを握りつぶしているだけですから!!!!!!!

(とくにJavaの世界においては)非検査例外というのはコーディングバグを示してくれるものですから、へんなnullチェックをいれるのではなく、積極的にNPEを発生させていきましょう。

こういうわけのわからないnullチェックを1箇所いれるとnullチェックをしなければ行けない箇所が芋づる式に増えていくので結果的に全体としてクソコードになります。

nullがきたらダメなところはNPEで落とそう。そのほうが安全だよ。


2017/06/05 追記

おもったよりたくさんいいね頂いた&上記のコード例だけだと誤解を招くので、ちょっと補足です。

上記のコード例は「メソッド(関数)を作るときに、それを使う人がnullを渡しちゃうかもしれないからnullでも処理が続行できるようにしようね!」っていう文脈で紹介されてました。元記事のコードはjavaではなく最近盛り上がっている某言語(Kotlinではないです)でした。

例外処理の考え方については、以下の記事を読んでいただきたく。。

http://qiita.com/Kokudori/items/2e4bd32abf7abea3186f


成功とは「関係データの不変条件が常に成り立ち、事前条件が成り立ち、処理が呼ばれ、事後条件が成り立つ」場合を指します。


上の例でhoge == nullだった場合、nullを渡してはいけないところにnullを渡しているので事前条件が成り立っていません(し、事後条件も成り立っていません)。処理は失敗していますが、呼び出し側がhoge.doSomething()の結果を検査するまで失敗に気づきません。(気づくためにはさらなるなんとかチェックが必要になります)危険なnullチェックにより失敗が検知できず、無駄なチェックを増やす必要を生じてしまうのでこのようなnullチェックは良くないね、っていうのが本稿の趣旨でございました。

(ユーザの入力値バリデーションなど)nullチェックを実行して然るべきケースはままありますので混同なきようお願いします!


2017/06/22 追記

たくさんのいいねとコメントありがとうございます。びっくりしてます

コメントを見ていると、「アプリケーション自体は継続すべきでは?」というご意見が散見されますのでそれについて補足しておきます。

本稿では「あるメソッドの事前条件が満たされなかった場合、その問題を呼び出し側に通知せず、あたかも事後条件がみたされたかのような振る舞いをするのはやめましょう」という主旨で処理を落とそう(=例外を発生させよ)と主張しているに過ぎず、そのメソッドを利用したアプリケーション自体のふるまいについては言及していない1つもりです。誤解を招く表現になっており申し訳ございません。





  1. アプリケーション自体を落とすべきではないという点に関しては僕も同意見です。