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.
/**
*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; }
}
/**
*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;
}
}
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.
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.
/**
*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; }
}
/**
*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;
}
}
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)
Recommended Posts