As a new graduate, I made an in-house goal management web application, but after all I learned Java and Play framework. There are some parts of the code that are pretty poorly visible and some that are refactorable.
Here is the content of Effective Java while shamefully exposing the refactoring I wanted to get a practical touch on what is written in the design pattern. It's a poor code, but if you actually refactor it, the readability of the code will increase so much! I would like beginners to realize that.
Also, with intermediate people, I expect Masakari who is like this in this part. The introduction is around here and go to the main subject!
This time I will fix the code of the part that generates CSV. Let's take a look at the code before fixing it. Think about where you would fix it.
public List<Csv> getEvaluations(String gaUserId, Integer year, String term) throws IOException {
AlibabaApi alibabaApi = AlibabaApiHelper.getAlibabaRet();
List<User> subordinates = alibabaApi.getSubordinates(gaUserId).execute().body().users;
List<User> mySubordinates = (subordinates.stream().filter(distinctByKey(p -> p.gaUserId)).collect(Collectors.toList()));
List<UserTerm> list = mySubordinates.stream().map(user -> termTargetService.getUserTerm(user.gaUserId,year,term)).collect(Collectors.toList());
List<Csv> allSubordinatesCsv = list.stream()
.map(user -> new Csv(
user.name,
user.grade,
user.termTargets.target == null ? "" : user.termTargets.target.body,
user.termTargets.target == null ? "" : user.termTargets.target.selfComments == null ? "" : user.termTargets.target.selfComments.quarter == null ? "" : user.termTargets.target.selfComments.quarter.body,
user.termTargets.contribution == null ? "" : user.termTargets.contribution.body,
user.termTargets.contribution == null ? "" : user.termTargets.contribution.selfComments == null ? "" : user.termTargets.contribution.selfComments.quarter == null ? "" : user.termTargets.contribution.selfComments.quarter.body,
user.termTargets.superiorComments == null ? "" : user.termTargets.superiorComments.quarter == null ? "" : user.termTargets.superiorComments.quarter.body)
).collect(Collectors.toList());
return allSubordinatesCsv;
}
Let's be clear. that's terrible. The purpose of this CSV is to give individual goals (monthly) and reflections on the goals for the number of people under management. The specific problem is
The point is that the outlook is very poor because there are many arguments in the part where new Csv () is done.
In addition, although it is convenient for the site, there is a great possibility that this CSV item will increase in the future, and it will be more difficult to read when the item increases as it is now. The code will be poorly maintained.
So I borrowed wisdom from Effective Java. This time we will use the Builder pattern.
The Builder pattern is usually new and an instance is created. Prepare a Builder method in a static class and set the value of the field.
The advantage of using the Builder pattern is that the visibility of the parameters passed to the constructor is much better. Also, if there are unnecessary parameters, you only have to set them, so you can guarantee the extensibility of the implementation.
See the section below for what you actually do! By the way, the name of the variable is changed for convenience.
berfor after Now let's actually refactor.
First, let's refactor the model class when creating csv. befor
public class Csv {
/**
*name
*/
public String name;
/**
*Position
*/
public String yakushoku;
/**
*Target
*/
public String mokuhyou;
/**
*Looking back on the goal
*/
public String hurikaeri;
/**
*Boss's comment on the goal
*/
public String joushinoKoment;
/**
*Goals for organizational contribution
*/
public String sosikikoukenMokuhyou;
/**
*Looking back on organizational contribution goals
*/
public String sosikikoukenMokuhyoHurikaeri;
public Csv(String name, String yakushoku, String mokuhyou, String hurikaeri, String joushinoKoment, String sosikikoukenMokuhyou,String sosikikoukenMokuhyoHurikaeri){
this.name = name;
this.grade = yakushoku;
this.target = mokuhyou;
this.hurikaeri = hurikaeri;
this.joushinoKoment = joushinoKoment;
this.sosikikoukenMokuhyou = sosikikoukenMokuhyou;
this.sosikikoukenMokuhyoHurikaeri = sosikikoukenMokuhyoHurikaeri;
}
}
after
public class Csv {
/**
*name
*/
public String name;
/**
*Position
*/
public String yakushoku;
/**
*Target
*/
public String mokuhyou;
/**
*Looking back on the goal
*/
public String hurikaeri;
/**
*Boss's comment on the goal
*/
public String joushinoKoment;
/**
*Goals for organizational contribution
*/
public String sosikikoukenMokuhyou;
/**
*Looking back on organizational contribution goals
*/
public String sosikikoukenMokuhyoHurikaeri;
private Csv() {
}
public static class Builder {
String name = "";
String yakushoku = "";
String mokuhyou = "";
String hurikaeri = "";
String joushinoKoment ="";
String sosikikoukenMokuhyou = "";
String sosikikoukenMokuhyoHurikaeri = "";
public Builder name(String name) {
this.name = name;
return this;
}
public Builder yakushoku(String yakushoku) {
this.yakushoku = yakushoku;
return this;
}
public Builder mokuhyou(String mokuhyou) {
this.mokuhyou = mokuhyou;
return this;
}
public Builder hurikaeri(String hurikaeri) {
this.hurikaeri = hurikaeri;
return this;
}
public Builder joushinoKoment(String joushinoKoment) {
this.joushinoKoment = joushinoKoment;
return this;
}
public Builder sosikikoukenMokuhyou(String sosikikoukenMokuhyou){
this.sosikikoukenMokuhyou = sosikikoukenMokuhyou;
return this;
}
public Builder sosikikoukenMokuhyoHurikaeri(String sosikikoukenMokuhyoHurikaeri) {
this.sosikikoukenMokuhyoHurikaeri = sosikikoukenMokuhyoHurikaeri;
return this;
}
public Csv build() {
Csv csv = new Csv();
csv.name = this.name;
csv.yakushoku = this.yakushoku;
csv.mokuhyou = this.mokuhyou;
csv.hurikaeri = this.hurikaeri;
csv.joushinoKoment = this.joushinoKoment;
csv.sosikikoukenMokuhyou = this.sosikikoukenMokuhyou;
csv.sosikikoukenMokuhyoHurikaeri = this.sosikikoukenMokuhyoHurikaeri;
return csv;
}
}
}
What I did specifically was to prepare a Csv type builder method.
And if the refactoring goes well, the actual CSV generation part will look like this.
private List<Csv> transformIntoCSV(List<UserTerm> list) {
return list.stream()
.map(user -> new Csv.Builder()
.name(user.name)
.grade(user.grade)
.target(user.termTargets.target == null ? "" : user.termTargets.target.body)
.commentForTarget(user.termTargets.target == null ? "" : user.termTargets.target.selfComments == null ? "" : user.termTargets.target.selfComments.quarter == null ? "" : user.termTargets.target.selfComments.quarter.body)
.contribution(user.termTargets.contribution == null ? "" : user.termTargets.contribution.body)
.commentForContribution(user.termTargets.contribution == null ? "" : user.termTargets.contribution.selfComments == null ? "" : user.termTargets.contribution.selfComments.quarter == null ? "" : user.termTargets.contribution.selfComments.quarter.body)
.superiorCommentForTarget(user.termTargets.superiorComments == null ? "" : user.termTargets.superiorComments.quarter == null ? "" : user.termTargets.superiorComments.quarter.body)
.build()
).collect(Collectors.toList());
}
The visibility of the code has improved considerably, and the code has become easier to maintain.
This time, using the Builder pattern that appears in Effective Java I tried to refactor the CSV generation part, which is difficult to read and maintain.
As a point, by using the Builder pattern, readability becomes high even if the number of arguments increases. The point is that the outlook for CSV items is improved.
I hope you've found that the Builder pattern is very effective in dealing with methods that require a lot of arguments because the requirements are easily changed like this time.
Recommended Posts