関数呼び出しと条件分岐を分離できないか - 良いコード書くきっかけを与えたい。9[C#リファクタリングサンプル]

#条件分岐後に呼ばれる関数
たまに、if文の中で呼ばれる関数は同じで引数だけ異なるコードを見かける。
本当にif文の中で呼ぶ必要があるのかについて、考えてみる。

##サンプルコード
ログ管理画面の例を考える。システム管理者には詳細なログ情報を表示するが、システム管理者以外には詳細なログを表示しないようにしている。

        public void Main(bool isManager, string detail)
        {
            // 管理者には詳細ログを表示する。
            if (isManager)
            {
                SetSummary("想定外エラーが発生しました。詳細ログを確認してください。");
                SetDetail(detail);
            }
            else
            {
                SetSummary("想定外エラーが発生しました。");
                SetDetail("詳細は管理者に問い合わせてください。");
            }
        }

        private void SetSummary(string summary)
        {
            System.Console.WriteLine("画面に概要を設定する。" + summary);
        }

        private void SetDetail(string detail)
        {
            System.Console.WriteLine("画面に詳細を設定する。" + detail);
        }

java

    public void main(boolean isManager, String detail) {
        // 管理者には詳細ログを表示する。
        if (isManager) {
            setSummary("想定外エラーが発生しました。詳細ログを確認してください。");
            setDetail(detail);

        } else {
            setSummary("想定外エラーが発生しました。");
            setDetail("詳細は管理者に問い合わせてください。");
        }
    }

    private void setSummary(String summary) {
        System.out.println("画面に概要を設定する。" + summary);
    }

    private void setDetail(String detail) {
        System.out.println("画面に詳細を設定する。" + detail);
    }

##気になること
このコードで気になるのは、if文の中で実際に異なるのは引数だけで関数呼び出しが同じという点だ。引数だけを変えて、関数呼び出しを一つにするほうがスッキリする。

##リファクタリング後

            // 管理者向けには詳細ログを表示する。
            string summary = null;
            string detail = null;
            if (isManager)
            {
                summary = "想定外エラーが発生しました。詳細ログを確認してください。";
                detail = detailLog;
            }
            else
            {
                summary = "想定外エラーが発生しました。";
                detail = "詳細は管理者に問い合わせてください。";
            }

            // 関数呼び出しの部分を条件分岐から分離した。
            SetSummary(summary);
            SetDetail(detail);

java

    public void main(boolean isManager, String detailLog) {
        // 管理者には詳細ログを表示する。
        String summary = null;
        String detail = null;
        if (isManager) {
            summary = "想定外エラーが発生しました。詳細ログを確認してください。";
            detail = detailLog;

        } else {
            summary = "想定外エラーが発生しました。";
            detail = "詳細は管理者に問い合わせてください。";
        }

        setSummary(summary);
        setDetail(detail);
    }

    private void setSummary(String summary) {
        System.out.println("画面に概要を設定する。" + summary);
    }

    private void setDetail(String detail) {
        System.out.println("画面に詳細を設定する。" + detail);
    }

##まとめ
全ての場合に、関数呼び出しを分離できるわけではないが、検討する余地はあると思う。この修正で何がうれしいかと言うと、メッセージ生成処理とメッセージ設定処理を分離できている点である。
例えば、メッセージ生成にさらに分岐が増えて、異なるメッセージを出すように修正することがあっても、後続のメッセージ設定処理は必ず呼ばれることが分かる。

前の記事(関数の引数が多すぎる)

次の記事(Getter、Setter逆問題)

目次