实践中的重构24_持续的方法重构

很少有人可以一遍就写出好的代码。写代码和写文章差不多，大部分人不是天才，需要的是不断的修改，才能使之演化为一段不错的代码。同时，即使是一段原本还算不错的代码，随着时间的推移，受到诸如需求变更，缺陷修复，后期维护等因素的影响，如果不是特别注意的话，也会使其慢慢腐烂。我们能做的，就是持续重构。
以下是一个例子。
原有的代码如下：

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;
	}

当然，这段代码是称不上完美的，但是和最初的版本相比，可以说，通过持续的重构，代码的质量还是得到了一个比较大的提高。
同时，还有一个有趣的现象，引入了多个小方法，代码行数竟然可以没有什么大的增加，这要归功于去除重复和重构过程中发现的更好的实现方案。