その「Nullチェック」必要ですか？ ～レビューで指摘を一つ減らすために～

プログラミングを初めて最初のほうで考えることの一つに、「Nullチェック」があります。
誰でも一度は、
「ここNullになりそうだけど大丈夫？」
とレビューで指摘をうけたことがあるでしょう。

それで覚えるやり方が、

if (hoge != null) {
    hoge.fuga();
}

こんな感じのやつですね。
ただ、覚えたての人って何でもかんでもこのコードを差し込みがちになると感じています。
安全安心なコードとして「Nullチェック」はとても大切ですが、
私としては、一緒に覚えてほしいことがあります。
「Nullチェック」の必要のないコーディングをすること
必要のないチェックはコードのステップ数が増えて煩雑さを生みます。
そして、チェックが必要なコードというのは作ったその時はわかっているのかもしれませんが、
チームのほかの人や未来の自分がそのモジュールを使ったときにNullチェックをし損ねて、
バグを出してしまう可能性があることを意味します。
チェックの必要のないコードはその時だけでなく周りや未来の自分にも優しいのです。

どの言語でも同じではありますが、
私がいまAndroidを中心に開発をしているのでjavaベースで書かせていただきます。

「Nullチェック」は必要な場所だけでやること

戻り値がCollectionの時にNullは返さない

よく「Nullチェック」をする個所としてメソッドの戻り値に対してのものが多いかと思います。

// DBから一覧を取得する
List<HogeEntity> hogeList = loadDbAll();
if (hogeList != null) {
    for (HogeEntity hoge : hogeList) {
        //処理
    }
}

こんな感じですね。
ただ、考えてみてください。
この「Nullチェック」は本当に必要でしょうか？

List<HogeEntity> loadDbAll() {
    ・
    ・処理
    ・
    if(件数が0件だったら) {
        return null;
    }
}

のような形になっていたら呼び出し側で「Nullチェック」は必要でしょう。
ただ、件数が0件であるという戻り値を返したいのであれば、Nullよりも空のCollectionを返したほうが、
状況としても適切ですし、呼び出し側で「Nullチェック」の必要もありません。

List<HogeEntity> loadDbAll() {
    ・
    ・処理
    ・
    if(件数が0件だったら) {
        return new List<HogeEntity>();
    }
}

こんな感じですね。
事前にCollectionを宣言していればそもそも件数チェックのif文すら不要になるかもしれません。

実行の成否をNullで表現しない

上記は全件取得のようなパターンでしたが、
特定のキーから1件だけ取得するという場合で考えてみます。
こういうときでも、取得出来たら～～するというように、「Nullチェック」することが多いと思います。

// DBから1件取得する
HogeEntity hoge = loadDb(id);
if (hoge != null) {
    hoge.fuga();
}

こんな形になると思います。
これも本当に必要でしょうか？
ユーザー入力からのあいまい検索をかけるような場合であればまだしも、
DBのPKなどを使ってシステム上で検索をかけるような、
狙ったレコードがヒットすることが前提である場合にはそれが取得できないというパターンは、
システム的な異常である可能性が高いです。
それにもかかわらず、取得できなかった時にNullを返すような作りは、
呼び出し改装にNullチェックをさせるだけでなく、システム異常の検知を投げることになるため設計上もよくありません。

List<HogeEntity> loadDb(int id) throws IllegalArgumentException {
    ・
    ・処理
    ・
    if(ヒットしなかったら) {
        throw new IllegalArgumentException();
    }
}

としておきましょう。
こうすることで、呼び出し側では「Nullチェック」の必要はなく、
実行の成否の判断もExceptionの発生で判断することができます。

Nullになる可能性のあるfindViewByIdはしない

Android開発をやっていると時々目にするコードに、

Button button = (Button)findViewById(R.id.my_button);
if (button != null) {
    button.setText();
}

のようなものがあります。
レビューでなぜこのタイミングで「Nullチェック」をするのか聞くと、
「複数の画面と共通化していて、このビューがない画面もあるからです」
と言われることが多いのですが、
「このビューがない画面であるならば、この処理を持ち込まないでほしい」と考えます。
Activityの親クラスを作って共通化していると思いますが、
継承関係を整理して必要なモノだけを継承するように設計しましょう。

おわり

以上、「Nullチェック」を減らすことでステップ数を減らし可読性を向上することができます。
また、Nullになると落ちるからとむやみにチェックを入れることでその場は回避できてなんとなく動くようになったとしても、本質的な問題に気づけないということに陥る可能性が残ってしまいます。
私見も多分に含まれていると思いますが、少しでもお役に立てば幸いです。