4
7

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

ちょっと気になるコード集-良いコード書くきっかけを与えたい。2[C#リファクタリングサンプル]

Last updated at Posted at 2017-02-11

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

  • Null許容型の使い方
  1. 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型
  1. キャスト
            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)

目次

4
7
6

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
4
7

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?