安易なNullチェック- 良いコード書くきっかけを与えたい。6[C#リファクタリングサンプル]

#安易なNullチェックの追加の妥当性
スプレッドのセルに値を代入する処理で、NullReferenceExceptionが発生した場合に書かれたコードについて考える。
Nullチェックを深く考えずに追加していたが、いろいろ考えるべきところがある。

##サンプルコード（例外発生前)
DBから取得した値をスプレッドに表示するコードで、値はnullの場合とそうでない場合が存在する。
スプレッドはサードパーティ製品のため、独自のクラスに置き換えている。
セルに値を代入する箇所で、ある日NullReferenceExceptionが発生した。

        public void Main(SpreadCell cell)
        {
            int? dbValue = null;
            // DBから値を取得する処理は省略

            // NullReferenceExceptionが発生する。
            cell.Value = dbValue.Value;
            
        }

        /** スプレッドのセルオブジェクト  **/
        private class SpreadCell
        {
            public Object Value { get; set; }
        }

java

    public void main(SpreadCell cell) {
        Integer dbValue = null;
        // DBから値を取得する処理は省略

        // NullPointExceptionが発生する。
        cell.setValue(dbValue.intValue());
    }

##サンプルコード(例外発生後に対応したコード)
Nullチェックを何も考えずに追加している上に、elseが無い。
何よりもNullチェックが必要ないということに気づいてほしい。

        public void Main(SpreadCell cell)
        {
            int? dbValue = null;
            // DBから値を取得する処理は省略

            // Null以外の場合だけ値を代入する。
            if (dbValue != null)
            {
                cell.Value = dbValue.Value;
            }          
        }

java

    public void main(SpreadCell cell) {
        Integer dbValue = null;
        // DBから値を取得する処理は省略

        // Null以外の場合だけ値を代入する。
        if (dbValue != null) {
            cell.setValue(dbValue.intValue());
        }
    }

##リファクタリング後のコード
Null許容型はそのまま代入すれば問題ない。百歩譲ってnullチェックをするならば、セルの初期化コードを入れてほしい。

        public void Main(SpreadCell cell)
        {
            int? dbValue = null;
            // DBから値を取得する処理は省略

            // そのまま代入すればよい。
            cell.Value = dbValue;

            // if文を使用するのであれば、初期化してほしい。
            cell.Value = null;
            if (dbValue != null)
            {
                cell.Value = dbValue.Value;
            }
        }

java

    public void main(SpreadCell cell) {
        Integer dbValue = null;
        // DBから値を取得する処理は省略

        // そのまま代入すればよい。
        cell.setValue(dbValue);

        // if文を使用するのであれば、初期化してほしい。
        cell.setValue(null);
        if (dbValue != null) {
            cell.setValue(dbValue.intValue());
        }
    }

##まとめ
NullReferenceExceptionが発生したからといって、何も考えずにNullチェックを追加するべきではない。
もし、Nullチェックが必要なのであれば、初期化コードも忘れずに。

こういう障害対応する人の場合、他のコードもちゃんと修正しているのか不安になる。

前の記事(コレクションに対する無駄な処理)

[次の記事(Bool型の扱い)]
(http://qiita.com/csharpisthebest/items/b0c44764948e1334e94a)

目次