ちょっと気になるコード集

コードレビューしていた中で、気になるソースがあったので挙げてみる。たいしたことではないが、気になってしょうがない。

Null許容型の使い方

Nullチェック

          int? inputValue = null;

          // NG こんな実装方法は思いつかなかったので、少しだけすごいと思ってしまった。
          if (string.IsNullOrWhiteSpace(inputValue.ToString()))
          {
          }

          // OK プロパティを使う
          if (!inputValue.HasValue)
          {
          }

          // OK !Nullable.HasValueと等価なため、これでもよい。
          if (inputValue == null)
          {
          }

　
2. Null許容型からの値取り出し。

            int? inputValue = 1;

            int value;
            if (inputValue.HasValue)
            {
                // NG 何でも文字列にすれば良いという考えは止めよう
                value = int.Parse(inputValue.ToString());
                // NG 無駄なキャスト1
                value = Convert.ToInt32((object)inputValue);
                // OK プロパティを使用
                value = inputValue.Value;
                // OK 無駄なキャストに見えるが、inputValue.Valueが呼ばれる。
                value = (int)inputValue;

            }

Object型

キャスト

            object objectValue = 1;
            int castValue;
            if (objectValue != null)
            {
                // NG 文字列にしてキャストするのは無駄が多い
                castValue = int.Parse(objectValue.ToString());
                // OK .Net Frameworkで用意された変換処理を使う
                castValue = Convert.ToInt32(objectValue);
                // OK 通常のキャスト
                castValue = (int)objectValue;
            }

まとめ

文字列にすれば良いと考えている人が多い。
動作としては問題なくても、レビューする側は気になってしょうがない。

サンプルコード(Java) 2017/3/24 追記

        /** Nullチェック **/
        Integer inputValue = null;

        // NG javaでこのような書き方をする人はいないだろう。
        // ちなみに、C#のint?はNullable構造体のため、ToStringする前のNullチェックは不要だった。
        if (inputValue == null || inputValue.toString().length() == 0) {

        }

        // OK Javaの場合、nullで比較するのが正しい。
        if (inputValue == null) {

        }
        /** Null許容型 **/
        Integer inputValue = 1;

        int value;
        if (inputValue != null) {
            // NG 何でも文字列にすれば良いという考えは止めよう
            value = Integer.parseInt(inputValue.toString());
            // NG 無駄なキャスト
            value = (int) inputValue;
            // OK Integer.intValueが呼ばれる。
            value = inputValue;
            // OK
            value = inputValue.intValue();
        }

        /** Object型 **/
        Object objectValue = 1;
        int castValue;
        if (objectValue != null) {
            // NG 元の型が分かっている場合、文字列に変換する必要がない。
            castValue = Integer.parseInt(objectValue.toString());
            // OK 元の型へキャスト
            castValue = (int) objectValue;
        }

前の記事(ややこしい条件分岐)

[次の記事(流れるようなインターフェース)]
(http://qiita.com/csharpisthebest/items/403d40374acc70ef24e3)

目次