When the processing after conditional branching is redundant-12 [C # refactoring sample]

When the processing after conditional branching is redundant

We often see cases where the same code exists in the processing after branching with an if statement. In particular, when writing the registration process by clicking the registration button, there are many processes that convert screen information to DTO. So, let's consider the sample code in this example.

Sample code

        /** 
         *Convert screen information to Dto.
         *Id and name assume the value of the Text property of Label and TextBox.
         *The id is null for new registration.
         **/
        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;
            }
        }

        /**Get the Dto from the table. Code omitted**/
        private Dto GetDto(int id)
        {
            return new Dto();
        }

        /**Table Dto. If new, id is-Set to 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
    /**
     *Convert screen information to Dto.
     *Id and name assume the value of the Text property of Label and TextBox.
     *The id is null for new registration.
     **/
    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;
    }

    /**Get the Dto from the table. Code omitted.**/
    private Dto getDto(int id) {
        //Get the Dto from the table. Code omitted.
        return new Dto();
    }

    /**Table Dto. If new, id is-Set to 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;
        }
    }

problem

I understand that the processing is divided between new and updated cases, but there are some of the same code. For example, if a phone number is added, you have to add two codes to assign the phone number to Dto with this code. This problem can be solved by separating only the processes that should be branched by the if statement.

Refactoring restrictions

Describe the restrictions on the refactoring policy. If there are no restrictions, there are many ways to fix it, so I want to avoid deviating from the main subject.

  1. To make it easier to understand the correspondence of the code, do not make it a function.
  2. The screen information should be ViewModel, etc., and the assumptions are not changed.
  3. Make no major changes to the code.

After refactoring

       /** 
         *Convert screen information to Dto.
         *Id and name assume the value of the Text property of Label and TextBox.
         *The id is null for new registration.
         **/
        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;
        }

        /**Get the Dto from the table. Code omitted**/
        private Dto GetDto(int id)
        {
            return new Dto();
        }

        /**Table Dto. If new, id is-Set to 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
    /**
     *Convert screen information to Dto.
     *Id and name assume the value of the Text property of Label and TextBox.
     *The id is null for new registration.
     **/
    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;
    }

    /**Get the Dto from the table. Code omitted.**/
    private Dto getDto(int id) {
        return new Dto();
    }

    /**Table Dto. If new, id is-Set to 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;
        }
    }

Summary

Maintainability has been improved by separating the code that depends on new and updates from the rest. In many cases, it is in a state like the sample code, or it is often divided into a new function and an update function. Please review if there is any redundant code before making it a function.

This refactoring viewpoint is the same as another article "Can function calls and conditional branching be separated?".

Previous article (Arrange everything)

Next article (Problems using screen display values)

Table of Contents

Recommended Posts

When the processing after conditional branching is redundant-12 [C # refactoring sample]
Whether conditional branching uses early returns or is safely written in conditional coverage-15 [C # refactoring sample]
Anything Array-11 [C # Refactoring Sample]
When the character string passed to C ++ by JNA is garbled
When will you do the refactoring?
Change the processing when the button on RecyclerView is pressed for each list
When the appearance of the input form is broken on the page after rendering