##条件分岐後の処理が冗長な場合
if文で分岐した後の処理で同じコードが存在しているケースをよく見かける。特に、登録ボタンクリックに登録処理を書く場合、画面の情報をDTOに変換する処理で多い。
そこで、この例でサンプルコードを考えてみる。

##サンプルコード

        /** 
         * 画面の情報をDtoに変換する。
         * IdやnameはLabelやTextBoxのTextプロパティの値の想定。 
         * idは新規登録はnullになっている。
         **/
        public Dto CreateDto(string id, string name, string updateUserId)
        {
            if (id == null)
            {
                var dto = new Dto();
                dto.Id = -1;
                dto.Name = name;
                dto.RegistUser = updateUserId;
                dto.RegistDate = DateTime.Now;
                dto.UpdateUserId = updateUserId;
                dto.UpdateDate = DateTime.Now;
                return dto;
            }
            else
            {
                var dto = GetDto(int.Parse(id));
                dto.Name = name;
                dto.UpdateUserId = updateUserId;
                dto.UpdateDate = DateTime.Now;
                return dto;
            }
        }

        /** テーブルからDtoを取得する。コードは省略 **/
        private Dto GetDto(int id)
        {
            return new Dto();
        }

        /** テーブルのDto。新規の場合、idは-1にする。 **/
        public class Dto
        {
            public int Id { get; set; }
            public string Name { get; set; }
            public string RegistUser { get; set; }
            public DateTime? RegistDate { get; set; }
            public string UpdateUserId { get; set; }
            public DateTime? UpdateDate { get; set; }
        }

java

    /**
     * 画面の情報をDtoに変換する。
     * IdやnameはLabelやTextBoxのTextプロパティの値の想定。
     * idは新規登録はnullになっている。
     **/
    public Dto createDto(String id, String name, String updateUserId) {
        Dto dto = null;

        if (id == null) {
            dto = new Dto();
            dto.setId(-1);
            dto.setName(name);
            dto.setRegistUser(updateUserId);
            dto.setRegistDate(new Date());
            dto.setUpdateUserId(updateUserId);
            dto.setUpdateDate(new Date());
        } else {
            dto = getDto(Integer.parseInt(id));
            dto.setName(name);
            dto.setUpdateUserId(updateUserId);
            dto.setUpdateDate(new Date());
        }

        return dto;
    }

    /** テーブルからDtoを取得する。コードは省略。 **/
    private Dto getDto(int id) {
        // テーブルからDtoを取得する。コードは省略。
        return new Dto();
    }

    /** テーブルのDto。新規の場合、idは-1にする。 **/
    public class Dto {
        private int id;
        private String name;
        private String registUser;
        private Date registDate;
        private String updateUserId;
        private Date updateDate;

        public int getId() {
            return id;
        }

        public void setId(int id) {
            this.id = id;
        }

        public String getName() {
            return name;
        }

        public void setName(String name) {
            this.name = name;
        }

        public String getRegistUser() {
            return registUser;
        }

        public void setRegistUser(String registUser) {
            this.registUser = registUser;
        }

        public Date getRegistDate() {
            return registDate;
        }

        public void setRegistDate(Date registDate) {
            this.registDate = registDate;
        }

        public String getUpdateUserId() {
            return updateUserId;
        }

        public void setUpdateUserId(String updateUserId) {
            this.updateUserId = updateUserId;
        }

        public Date getUpdateDate() {
            return updateDate;
        }

        public void setUpdateDate(Date updateDate) {
            this.updateDate = updateDate;
        }
    }

##問題点
新規、更新の場合で処理を分けているのは分かるが、同じコードがいくつか存在している。例えば、電話番号が追加された場合、このコードだとDtoへ電話番号を代入するコードを２箇所追加しなければならない。if文で分岐するべき処理だけを分離すれば、この問題は解消される。

##リファクタリング　制限事項
リファクタリング方針について、制限事項を記載しておく。制限事項がなければ、修正方法がいくつでもあるため、本題と逸れてしまうことを避けたい。

　1. コードの対応を分かりやすくするため、関数化はしない。
　2. 画面の情報はViewModelにするべきなど、前提を変える修正はしない。
　3. コードを大幅に変える修正はしない。

#リファクタリング後

       /** 
         * 画面の情報をDtoに変換する。
         * IdやnameはLabelやTextBoxのTextプロパティの値の想定。 
         * idは新規登録はnullになっている。
         **/
        public Dto CreateDto(string id, string name, string updateUserId)
        {
            Dto dto = null;

            if (id == null)
            {
                dto = new Dto();
                dto.Id = -1;
                dto.RegistUser = updateUserId;
                dto.RegistDate = DateTime.Now;
            }
            else
            {
                dto = GetDto(int.Parse(id));
            }

            dto.Name = name;
            dto.UpdateUserId = updateUserId;
            dto.UpdateDate = DateTime.Now;

            return dto;
        }

        /** テーブルからDtoを取得する。コードは省略 **/
        private Dto GetDto(int id)
        {
            return new Dto();
        }

        /** テーブルのDto。新規の場合、idは-1にする。 **/
        public class Dto
        {
            public int Id { get; set; }
            public string Name { get; set; }
            public string RegistUser { get; set; }
            public DateTime? RegistDate { get; set; }
            public string UpdateUserId { get; set; }
            public DateTime? UpdateDate { get; set; }
        }

java

    /**
     * 画面の情報をDtoに変換する。
     * IdやnameはLabelやTextBoxのTextプロパティの値の想定。
     * idは新規登録はnullになっている。
     **/
    public Dto createDto(String id, String name, String updateUserId) {
        Dto dto = null;

        if (id == null) {
            dto = new Dto();
            dto.setId(-1);
            dto.setRegistUser(updateUserId);
            dto.setRegistDate(new Date());
        } else {
            dto = getDto(Integer.parseInt(id));
        }

        dto.setName(name);
        dto.setUpdateUserId(updateUserId);
        dto.setUpdateDate(new Date());

        return dto;
    }

    /** テーブルからDtoを取得する。コードは省略。 **/
    private Dto getDto(int id) {
        return new Dto();
    }

    /** テーブルのDto。新規の場合、idは-1にする。 **/
    public class Dto {
        private int id;
        private String name;
        private String registUser;
        private Date registDate;
        private String updateUserId;
        private Date updateDate;

        public int getId() {
            return id;
        }

        public void setId(int id) {
            this.id = id;
        }

        public String getName() {
            return name;
        }

        public void setName(String name) {
            this.name = name;
        }

        public String getRegistUser() {
            return registUser;
        }

        public void setRegistUser(String registUser) {
            this.registUser = registUser;
        }

        public Date getRegistDate() {
            return registDate;
        }

        public void setRegistDate(Date registDate) {
            this.registDate = registDate;
        }

        public String getUpdateUserId() {
            return updateUserId;
        }

        public void setUpdateUserId(String updateUserId) {
            this.updateUserId = updateUserId;
        }

        public Date getUpdateDate() {
            return updateDate;
        }

        public void setUpdateDate(Date updateDate) {
            this.updateDate = updateDate;
        }
    }

#まとめ
新規、更新に依存するコードとそれ以外を分離することで、保守性が高くなった。
多くの場合、サンプルコードのような状態か、新規用関数と更新用関数に分ける場合が多い。
関数化するまえに冗長なコードが無いか見直してほしい。

尚、このリファクタリング観点は、別の記事「関数呼び出しと条件分岐を分離できないか」と同じである。

前の記事(なんでもかんでも配列)

次の記事(画面の表示値を利用することの問題点)

目次