很少有人可以一遍就写出好的代码。写代码和写文章差不多,大部分人不是天才,需要的是不断的修改,才能使之演化为一段不错的代码。同时,即使是一段原本还算不错的代码,随着时间的推移,受到诸如需求变更,缺陷修复,后期维护等因素的影响,如果不是特别注意的话,也会使其慢慢腐烂。我们能做的,就是持续重构。
以下是一个例子。
原有的代码如下:
该接口的实现如下:
原始的方法实现稍微有点长,在没有bug和需求变更的情况下,一直静静的躺着系统中。直到有一天,不可避免的,需求变更来临了。
新的需求比较简单。
新加入一个参数userId,通过该userId如果可以找到一个UserInfo的话,则返回结果的日期截止点为该UserInfo的最后修改日期。由其他系统保证该修改日期在当前时间之前的某一天。
新的实现如下:
可以看到,新的实现就是在原有的基础上直接修修补补,并没有经过重新设计。
新的实现有以下问题:
1 方法体变的更长了,这个原有的问题更加严重了。
2 UserInfo的定义和使用距离太远了,而在第一行就定义了UserInfo很容易给人一个错觉,该UserInfo和该方法关系很大,但是,实际上,UserInfo只是界定了结果日期的最后截止日期而已。
3 isEquals变量的意义不清晰。
4 补全list大小为7处的代码变得不清晰了。
先来看实现的一个改进,目前最大的问题就是方法体太长,充斥着细节,使其可读性和可维护性降低。解决该问题的思路就是分拆大方法为小方法,提高代码的抽象级别,并且使小方法专注于做一件事情。
新的实现如下:
原始的实现被分拆为3步,得到结果的所有日期,转化为周日期列表的列表,填充null。
同时,在该重构过程中,实现completeWeekListWithNull时发现了一个bug,当结果的列表大小为1的时候,原有的补充null实现是不正确的。意外收获啊,但是也是在情理之中,这也是方法专注干一件事情带来的好处。
当小方法关注于做一个小的事情时,由于关注点比较单一,边界比较清晰,因此更容易产出好的代码。
但是,解决该问题的同时,引入了新的问题。
首先,completeWeekListWithNull方法变长了,有的地方用日期来判断是否需要补充null,有的地方用size来判断是否需要补充null,同样一件事情用不同的方式去做,系统引入了不一致性。保证系统的一致性可以提高代码的可读性和可维护性,避免后来者浪费时间在不一致的理解上。
其次,处理一个list和处理多个list是使用了一个if判断来进行分支处理的。但是处理一个 list和处理多个list在这里真的就是截然不同的两种场景吗?这里的需求也可以描述为把每个list按照自然周用null补齐,而不是,当只有一个list时,怎样怎样,当有多个list时,怎样怎样。用一个简单的需求,简单的实现代替多余的条件判断可以简化代码。
不一致是要去除的,多余的条件判断也是要被替代掉的。
又一轮重构,新的实现如下:
恩,代码和谐多了。
但是又review了几遍后,还是觉得这个代码不应当是定稿。
1 比较明显的,isMonday和isSunday很像,代码又产生了重复。
2 completeWeekListWithNull中对每个子list都补充一遍null感觉比较怪。
3 convertToWeekList看起来还是费劲一些。
这次不是很容易了,经过了一些时间的思考,突然灵机一动,又审视了一遍原始的3步曲,得到结果的所有日期,转化为周日期列表的列表,填充null。convertToWeekList的复杂性在于,处理的是一个无规律的日期列表。如果把3步曲调整一下顺序,变为得到结果的所有日期,填充null,转化为周日期列表的列表。这样一箭双雕,解决了2和3的问题。
最后的实现如下:
当然,这段代码是称不上完美的,但是和最初的版本相比,可以说,通过持续的重构,代码的质量还是得到了一个比较大的提高。
同时,还有一个有趣的现象,引入了多个小方法,代码行数竟然可以没有什么大的增加,这要归功于去除重复和重构过程中发现的更好的实现方案。
以下是一个例子。
原有的代码如下:
public interface DateService {
/**
* 构建一个按照自然周排好的日期列表(包括最近30天,不包含当日),周时间距离现在越近,在列表中越靠前。
*
* <pre>
* 保证每个子列表的大小都为7(一个星期的天数),没有日期则以null填充。
*
* 例子:
* 如果当前时间是2010-09-08(星期3)则构建的列表为:
* 第1个subList 2010-09-06(星期1),2010-09-07(星期2),null,null,null,null,null,
* 第2个subList 2010-08-30(星期1),2010-08-31(星期2),2010-09-01(星期3),2010-09-02(星期4),2010-09-03(星期5),2010-09-04(星期6),2010-09-05(星期日)
* ...
* 第n个subList null,null,null,null,date(星期5),date(星期6),date(星期日)
* </pre>
*
*
* @return 返回构建好的列表。
* */
public List<List<Date>> getDaysInWeekList();
}
该接口的实现如下:
public class DateServiceImpl implements DateService {
private final static int DAYS_RANGE = 30;
private final static int DAYS_OF_WEEK = 7;
@Override
public List<List<Date>> getDaysInWeekList() {
List<List<Date>> result = new ArrayList<List<Date>>();
Calendar calendar = Calendar.getInstance();
calendar.add(Calendar.DATE, -DAYS_RANGE);
List<Date> subList = new ArrayList<Date>();
result.add(subList);
for (int i = 0; i < DAYS_RANGE; i++) {
Date date = calendar.getTime();
List<Date> recentList = result.get(0);
if (recentList.isEmpty()) {
recentList.add(date);
} else {
if (calendar.get(Calendar.DAY_OF_WEEK) == Calendar.MONDAY) {
List<Date> newRecentList = new ArrayList<Date>();
newRecentList.add(date);
result.add(0, newRecentList);
} else {
recentList.add(date);
}
}
calendar.add(Calendar.DATE, 1);
}
// 补全,使每个list大小为7。
List<Date> recentList = result.get(0);
int gap = DAYS_OF_WEEK - recentList.size();
for (int i = 0; i < gap; i++) {
recentList.add(null);
}
List<Date> lastList = result.get(result.size() - 1);
gap = DAYS_OF_WEEK - lastList.size();
for (int i = 0; i < gap; i++) {
lastList.add(0, null);
}
return result;
}
}
原始的方法实现稍微有点长,在没有bug和需求变更的情况下,一直静静的躺着系统中。直到有一天,不可避免的,需求变更来临了。
新的需求比较简单。
新加入一个参数userId,通过该userId如果可以找到一个UserInfo的话,则返回结果的日期截止点为该UserInfo的最后修改日期。由其他系统保证该修改日期在当前时间之前的某一天。
新的实现如下:
private final static int DAYS_RANGE = 30;
private final static int DAYS_OF_WEEK = 7;
private UserService userService;
@Override
public List<List<Date>> getDaysInWeekList(String userId) {
UserInfo userInfo = userService.findUserInfo(userId);
List<List<Date>> result = new ArrayList<List<Date>>();
Calendar calendar = Calendar.getInstance();
calendar.add(Calendar.DATE, -DAYS_RANGE);
List<Date> subList = new ArrayList<Date>();
result.add(subList);
for (int i = 0; i < DAYS_RANGE; i++) {
Date date = calendar.getTime();
List<Date> recentList = result.get(0);
if (recentList.isEmpty()) {
recentList.add(date);
} else {
if (calendar.get(Calendar.DAY_OF_WEEK) == Calendar.MONDAY) {
List<Date> newRecentList = new ArrayList<Date>();
newRecentList.add(date);
result.add(0, newRecentList);
} else {
recentList.add(date);
}
}
calendar.add(Calendar.DATE, 1);
}
// 若可以找到该用户信息。处理特殊情况。
if (null != userInfo) {
// 是否需要比较时间,默认true
boolean isEquals = true;
// 用户修改时间
Date modifyDate = DayUtil.getDayBegin(userInfo.getModifyDate());
for (List<Date> dateList : result) {
if (!isEquals) {
break;
}
for (int i = dateList.size() - 1; i >= 0; i--) {
// 日历时间
Date date = DayUtil.getDayBegin(dateList.get(i));
// 集合是按日历时间倒序迭代
if (!date.after(modifyDate)) {
isEquals = false;
break;
} else {
dateList.remove(i);
}
}
}
}
// 补全,使每个list大小为7。
List<Date> lastList = result.get(result.size() - 1);
int gap = DAYS_OF_WEEK - lastList.size();
for (int i = 0; i < gap; i++) {
lastList.add(0, null);
}
for (List<Date> dateList : result) {
int tempGap = DAYS_OF_WEEK - dateList.size();
for (int i = 0; i < tempGap; i++) {
dateList.add(null);
}
}
return result;
}
可以看到,新的实现就是在原有的基础上直接修修补补,并没有经过重新设计。
新的实现有以下问题:
1 方法体变的更长了,这个原有的问题更加严重了。
2 UserInfo的定义和使用距离太远了,而在第一行就定义了UserInfo很容易给人一个错觉,该UserInfo和该方法关系很大,但是,实际上,UserInfo只是界定了结果日期的最后截止日期而已。
3 isEquals变量的意义不清晰。
4 补全list大小为7处的代码变得不清晰了。
先来看实现的一个改进,目前最大的问题就是方法体太长,充斥着细节,使其可读性和可维护性降低。解决该问题的思路就是分拆大方法为小方法,提高代码的抽象级别,并且使小方法专注于做一件事情。
新的实现如下:
private final static int DAYS_RANGE = 30;
private final static int DAYS_OF_WEEK = 7;
private UserService userService;
@Override
public List<List<Date>> getDaysInWeekList(String userId) {
List<Date> dayList = getAllowedDateList(userId);
if (dayList.isEmpty()) {
return new ArrayList<List<Date>>();
}
List<List<Date>> weekList = convertToWeekList(dayList);
completeWeekListWithNull(weekList);
return weekList;
}
private List<Date> getAllowedDateList(String userId) {
Date startDate = DayUtil.getDayBegin(DayUtil.addDays(new Date(),
-DAYS_RANGE));
Date endDate = null;
UserInfo userInfo = userService.findUserInfo(userId);
if (userInfo != null) {
endDate = DayUtil.getDayBegin(userInfo.getModifyDate());
} else {
endDate = DayUtil.getDayBegin(DayUtil.addDays(new Date(), -1));
}
List<Date> list = new ArrayList<Date>();
for (Date tmpDate = startDate; !tmpDate.after(endDate); tmpDate = DayUtil
.addDays(tmpDate, 1)) {
list.add(tmpDate);
}
return list;
}
private List<List<Date>> convertToWeekList(List<Date> dateList) {
List<List<Date>> result = new ArrayList<List<Date>>();
result.add(new ArrayList<Date>());
for (Date temDate : dateList) {
List<Date> recentWeekList = result.get(0);
if (recentWeekList.isEmpty()) {
recentWeekList.add(temDate);
continue;
}
if (isMonday(temDate)) {
List<Date> newRecentWeekList = new ArrayList<Date>();
newRecentWeekList.add(temDate);
result.add(0, newRecentWeekList);
} else {
recentWeekList.add(temDate);
}
}
return result;
}
private void completeWeekListWithNull(List<List<Date>> weekList) {
int gap = -1;
if (weekList.size() == 1) {
List<Date> list = weekList.get(0);
Date startDate = list.get(0);
for (Date temDate = startDate; !isMonday(temDate); temDate = DayUtil
.addDays(temDate, -1)) {
list.add(0, null);
}
gap = DAYS_OF_WEEK - list.size();
for (int i = 0; i < gap; i++) {
list.add(null);
}
return;
}
List<Date> recentList = weekList.get(0);
gap = DAYS_OF_WEEK - recentList.size();
for (int i = 0; i < gap; i++) {
recentList.add(null);
}
List<Date> lastList = weekList.get(weekList.size() - 1);
gap = DAYS_OF_WEEK - lastList.size();
for (int i = 0; i < gap; i++) {
lastList.add(0, null);
}
}
private boolean isMonday(Date date) {
Calendar calendar = Calendar.getInstance();
calendar.setTime(date);
return calendar.get(Calendar.DAY_OF_WEEK) == Calendar.MONDAY;
}
原始的实现被分拆为3步,得到结果的所有日期,转化为周日期列表的列表,填充null。
同时,在该重构过程中,实现completeWeekListWithNull时发现了一个bug,当结果的列表大小为1的时候,原有的补充null实现是不正确的。意外收获啊,但是也是在情理之中,这也是方法专注干一件事情带来的好处。
当小方法关注于做一个小的事情时,由于关注点比较单一,边界比较清晰,因此更容易产出好的代码。
但是,解决该问题的同时,引入了新的问题。
首先,completeWeekListWithNull方法变长了,有的地方用日期来判断是否需要补充null,有的地方用size来判断是否需要补充null,同样一件事情用不同的方式去做,系统引入了不一致性。保证系统的一致性可以提高代码的可读性和可维护性,避免后来者浪费时间在不一致的理解上。
其次,处理一个list和处理多个list是使用了一个if判断来进行分支处理的。但是处理一个 list和处理多个list在这里真的就是截然不同的两种场景吗?这里的需求也可以描述为把每个list按照自然周用null补齐,而不是,当只有一个list时,怎样怎样,当有多个list时,怎样怎样。用一个简单的需求,简单的实现代替多余的条件判断可以简化代码。
不一致是要去除的,多余的条件判断也是要被替代掉的。
又一轮重构,新的实现如下:
private void completeWeekListWithNull(List<List<Date>> weekList) {
for (List<Date> list : weekList) {
completeWeekListToStart(list);
completeWeekListToEnd(list);
}
}
private void completeWeekListToStart(List<Date> list) {
if (list.size() == DAYS_OF_WEEK) {
return;
}
Date startDate = list.get(0);
for (Date temDate = startDate; !isMonday(temDate); temDate = DayUtil
.addDays(temDate, -1)) {
list.add(0, null);
}
}
private void completeWeekListToEnd(List<Date> list) {
if (list.size() == DAYS_OF_WEEK) {
return;
}
Date endDate = list.get(list.size() - 1);
for (Date temDate = endDate; !isSunday(temDate); temDate = DayUtil
.addDays(temDate, 1)) {
list.add(null);
}
}
private boolean isMonday(Date date) {
Calendar calendar = Calendar.getInstance();
calendar.setTime(date);
return calendar.get(Calendar.DAY_OF_WEEK) == Calendar.MONDAY;
}
private boolean isSunday(Date date) {
Calendar calendar = Calendar.getInstance();
calendar.setTime(date);
return calendar.get(Calendar.DAY_OF_WEEK) == Calendar.SUNDAY;
}
恩,代码和谐多了。
但是又review了几遍后,还是觉得这个代码不应当是定稿。
1 比较明显的,isMonday和isSunday很像,代码又产生了重复。
2 completeWeekListWithNull中对每个子list都补充一遍null感觉比较怪。
3 convertToWeekList看起来还是费劲一些。
这次不是很容易了,经过了一些时间的思考,突然灵机一动,又审视了一遍原始的3步曲,得到结果的所有日期,转化为周日期列表的列表,填充null。convertToWeekList的复杂性在于,处理的是一个无规律的日期列表。如果把3步曲调整一下顺序,变为得到结果的所有日期,填充null,转化为周日期列表的列表。这样一箭双雕,解决了2和3的问题。
最后的实现如下:
private final static int DAYS_RANGE = 30;
private final static int DAYS_OF_WEEK = 7;
private UserService userService;
@Override
public List<List<Date>> getDaysInWeekList(String userId) {
List<Date> dayList = getAllowedDateList(userId);
if (dayList.isEmpty()) {
return new ArrayList<List<Date>>();
}
completeDayListWithNull(dayList);
List<List<Date>> weekList = convertToWeekList(dayList);
return weekList;
}
private List<Date> getAllowedDateList(String userId) {
Date startDate = DayUtil.getDayBegin(DayUtil.addDays(new Date(),
-DAYS_RANGE));
Date endDate = null;
UserInfo userInfo = userService.findUserInfo(userId);
if (userInfo != null) {
endDate = DayUtil.getDayBegin(userInfo.getModifyDate());
} else {
endDate = DayUtil.getDayBegin(DayUtil.addDays(new Date(), -1));
}
List<Date> list = new ArrayList<Date>();
for (Date tmpDate = startDate; !tmpDate.after(endDate); tmpDate = DayUtil
.addDays(tmpDate, 1)) {
list.add(tmpDate);
}
return list;
}
private List<List<Date>> convertToWeekList(List<Date> dateList) {
List<List<Date>> result = new ArrayList<List<Date>>();
List<Date> weekList = new ArrayList<Date>();
result.add(0, weekList);
for (Date date : dateList) {
if (weekList.size() == DAYS_OF_WEEK) {
weekList = new ArrayList<Date>();
result.add(0, weekList);
}
weekList.add(date);
}
return result;
}
private void completeDayListWithNull(List<Date> dateList) {
completeDayListToMonday(dateList);
completeDayListToSunday(dateList);
}
private void completeDayListToMonday(List<Date> dateList) {
Date startDate = dateList.get(0);
for (Date temDate = startDate; !isDayOfWeek(temDate, Calendar.MONDAY); temDate = DayUtil
.addDays(temDate, -1)) {
dateList.add(0, null);
}
}
private void completeDayListToSunday(List<Date> dateList) {
Date endDate = dateList.get(dateList.size() - 1);
for (Date temDate = endDate; !isDayOfWeek(temDate, Calendar.SUNDAY); temDate = DayUtil
.addDays(temDate, 1)) {
dateList.add(null);
}
}
private boolean isDayOfWeek(Date date, int dayOfWeek) {
Calendar calendar = Calendar.getInstance();
calendar.setTime(date);
return calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek;
}
当然,这段代码是称不上完美的,但是和最初的版本相比,可以说,通过持续的重构,代码的质量还是得到了一个比较大的提高。
同时,还有一个有趣的现象,引入了多个小方法,代码行数竟然可以没有什么大的增加,这要归功于去除重复和重构过程中发现的更好的实现方案。