冗長な代入文 - 良いコード書くきっかけを与えたい。4[C#リファクタリングサンプル]

#冗長な代入文
プロジェクトによっては縦持ちのテーブルで設計するべきなのに、正規化されず横持ちになっていることがある。
例えば、親子の情報を登録する場合、両親がいて、子供が一人いる場合、人コードを振って、
{子供コード, 父親コード}, {子供コード,母親コード}
という形式でテーブルに登録するべきである。
しかし、なんらかの理由で
{子供コード,父親コード,母親コード}
と１レコードで表現したい場合がある。

ここでは、テーブルの形式ではなく、画面で子供と親のコードを登録するコードがどうなるかを考えたい。

##ソースコード
子供と親コード１、親コード２を入力して、登録する画面を考える。その場合、登録ボタンクリック処理は以下のようなコードを良く見かける。

        /** 登録ボタンクリック **/
        private void buttonRegist_Click(object sender, EventArgs e)
        {
            // 画面からの値をDTOに代入する。
            // 空文字やスペースはNULLとして登録する。
            var dto = new Company();
            if (string.IsNullOrWhiteSpace(textBoxChildCode.Text))
            {
                dto.ChildCode = null;
            }
            else
            {
                dto.ChildCode = textBoxChildCode.Text.Trim();
            }

            if (string.IsNullOrWhiteSpace(textBoxParentCode1.Text))
            {
                dto.ParentCode1 = null;
            }
            else
            {
                dto.ParentCode1 = textBoxParentCode1.Text.Trim();
            }

            if (string.IsNullOrWhiteSpace(textBoxParentCode2.Text))
            {
                dto.ParentCode2 = null;
            }
            else
            {
                dto.ParentCode2 = textBoxParentCode2.Text.Trim();
            }

            // DB登録処理は省略
        }

    /** DTO **/
    public class Company
    {
        public string ChildCode { get; set; }
        public string ParentCode1 { get; set; }
        public string ParentCode2 { get; set; }
    }

##気になること
空文字やスペースをNULLとして扱う部分が冗長なため、コードが長い。
この部分をルーチン化すれば、可読性があがる。

        private void buttonRegist_Click(object sender, EventArgs e)
        {
            // 画面からの値をDTOに代入する。
            // 空文字やスペースはNULLとして登録する。
            var dto = new Company();
            dto.ChildCode = GetDBValue(textBoxChildCode.Text);
            dto.ParentCode1 = GetDBValue(textBoxParentCode1.Text);
            dto.ParentCode2 = GetDBValue(textBoxParentCode2.Text);

            // DB登録処理は省略
        }

        /** 同じ処理はルーチンにする。 **/
        private string GetDBValue(string inputValue)
        {
            if (string.IsNullOrWhiteSpace(inputValue))
            {
                return null;
            }
            else
            {
                return inputValue.Trim();
            }
        }

###まとめ
冗長だからルーチン化できるのではないか、ということを考えてほしい。
それほど難しいことではないと思うものの、意外と思いつかないものらしい。
ちなみに、似たようなコードを書く人にルーチン化したコードを教えたところ「思いつかなかった」とのことだった。
また、同じ処理を何回も書いているという意味ではレガシーコードと言えるかもしれない。

[前の記事(流れるようなインターフェースの使い方はこれで正しいのか？)]
(http://qiita.com/csharpisthebest/items/403d40374acc70ef24e3)

[次の記事(コレクションに対する無駄な処理)]
(http://qiita.com/csharpisthebest/items/7a9177a2a27002579b03)

目次