实践中的重构10_平铺直叙的代码

    很多应用程序的主要目的就是用计算机来代替人处理真实世界中的问题。真实世界和计算机世界之间有着巨大的差异。编程语言，编程方法一直以来都有一个重要的发展方向，提供一个平台，尽量减少这个差异。面向对象语言，面向领域编程，为解决该问题在不同层次提供了解决方案。
    在代码级别，也可以通过种种方式减少这个差异。
先看下面一段代码，需求和场景是这样的。
    查询用户列表，传入一个整数，指定一次查询返回用户数的上限。
    有3个不同的数据源可以用来查询用户信息，这3个数据源在查询的时候是有优先级的。即先查询第1个数据源，如果得到的用户数少于上限，则到第2个数据源查询，如果还是少于上限，则到第3个数据源查询。

	public List<User> getUserList_0(int maxSize) {

		List<User> userList;
		int selectSize = 0;
		userList = getUserListFromSource1(maxSize);

		if (null != userList) {
			if (userList.size() < maxSize) {
				selectSize = maxSize - userList.size();
				List<User> userList2 = getUserListFromSource2(selectSize);

				if (null != userList2 && !userList2.isEmpty()) {
					userList.addAll(userList2);
					if (userList.size() < maxSize) {
						selectSize = maxSize - userList.size();
						List<User> userList3 = getUserListFromSource3(selectSize);
						if (null != userList3 && !userList3.isEmpty()) {
							userList.addAll(userList3);
						}
					}
				}
			}

		} else {
			userList = getUserListFromSource2(maxSize);
			if (userList.size() < maxSize) {
				selectSize = maxSize - userList.size();
				List<User> userList4 = getUserListFromSource3(selectSize);
				if (null != userList4 && !userList4.isEmpty()) {
					userList.addAll(userList4);
				}
			}
		}

		return userList;
	}

    如果你耐心的看完了这段代码，我只能说佩服了。一眼看到5层的if嵌套，我已经是半晕状态了。我承认，我没有耐心看完这段代码。
    该方法的需求和场景都是比较简单的，因此应该存在简单的写法。
重写的代码如下：

	public List<User> getUserList_1(final int maxSize) {

		List<User> userList = new ArrayList<User>();
		List<User> temList;

		int selectSize = maxSize - userList.size();
		temList = getUserListFromSource1(selectSize);
		if (temList != null) {
			userList.addAll(temList);
		}
		if (userList.size() >= maxSize) {
			return userList;
		}

		selectSize = maxSize - userList.size();
		temList = getUserListFromSource2(selectSize);
		if (temList != null) {
			userList.addAll(temList);
		}
		if (userList.size() >= maxSize) {
			return userList;
		}

		selectSize = maxSize - userList.size();
		temList = getUserListFromSource3(selectSize);
		if (temList != null) {
			userList.addAll(temList);
		}

		return userList;
	}

    这里首先对参数加上final修饰符，明确表明方法中不会修改该参数。其次，去除掉了if嵌套，用相似的代码结构试图更清晰更直白的表明这段代码的意图。
    这段代码有这样一些小的问题，可以持续改进。
    1 如果可以预期该方法返回的用户列表大小的话，可以设置初始的列表大小。
    2 如果遵循查询结果为空，返回空列表而不是null的话，可以是代码更紧凑。

	public List<User> getUserList_2(final int maxSize) {

		List<User> userList = new ArrayList<User>(maxSize);

		int selectSize = maxSize - userList.size();
		userList.addAll(getUserListFromSource1(selectSize));
		if (userList.size() >= maxSize) {
			return userList;
		}

		selectSize = maxSize - userList.size();
		userList.addAll(getUserListFromSource2(selectSize));
		if (userList.size() >= maxSize) {
			return userList;
		}

		selectSize = maxSize - userList.size();
		userList.addAll(getUserListFromSource3(selectSize));

		return userList;
	}

    这样看上去好多了。这里遵循的原则就是平铺直叙的用编程语言翻译需求，试图用接近自然语言的表达方式实现需求，从而减少两个世界的差异。
    更好的一种方式是先用自然语言（或者伪代码）描述需求的实现，然后翻译成真正的代码。
    用自然语言描述这个需求的实现如下：
    1 新建一个空的用户列表。
    2 计算下一次查询的上限。
    3 到数据源1去查询。
    4 添加结果到用户列表。
    5 如果用户列表达到上限则返回。
    6 计算下一次查询的上限。
    7 到数据源2去查询。
    8 添加结果到用户列表。
    9 如果用户列表达到上限则返回。
    10 计算下一次查询的上限。
    11 到数据源3去查询。
    12 添加结果到用户列表。
    13 返回。
    这个实现的描述和真实的实现很像，即使没有实现，看着这个自然语言的实现翻译成对应的代码实现也很容易。
    当然，计算机编程是复杂的，这里的实现还是有一个潜在的问题。代码中有重复的结构。鉴于这里的实现已经比较简单了，所以没有进一步去掉这个重复的结构。但是，如果需求改变，需要查询更多的数据源，或者可配置的数据源，代码可以做以下演进。

	interface UserQueryService {
		List<User> queryUserList(int maxSize);
	}

	class UserQueryServiceImpl_1 implements UserQueryService {

		@Override
		public List<User> queryUserList(int maxSize) {
			return getUserListFromSource1(maxSize);
		}
	}

	class UserQueryServiceImpl_2 implements UserQueryService {

		@Override
		public List<User> queryUserList(int maxSize) {
			return getUserListFromSource2(maxSize);
		}
	}

	class UserQueryServiceImpl_3 implements UserQueryService {

		@Override
		public List<User> queryUserList(int maxSize) {
			return getUserListFromSource3(maxSize);
		}
	}

	public List<User> getUserList_3(final int maxSize) {

		// 这里的userQueryServices可以从配置文件中生成，也可以用户传入。
		// 这里为了演示，硬编码一下。
		List<UserQueryService> userQueryServices = new ArrayList<UserQueryService>();
		userQueryServices.add(new UserQueryServiceImpl_1());
		userQueryServices.add(new UserQueryServiceImpl_2());
		userQueryServices.add(new UserQueryServiceImpl_3());

		List<User> userList = new ArrayList<User>(maxSize);

		// 方式1
		for (UserQueryService service : userQueryServices) {
			int selectMaxSize = maxSize - userList.size();
			userList.addAll(service.queryUserList(selectMaxSize));

			if (userList.size() >= maxSize) {
				break;
			}
		}

		// 方式2
		for (int i = 0; i < userQueryServices.size()
				&& userList.size() < maxSize; i++) {
			UserQueryService service = userQueryServices.get(i);
			int selectMaxSize = maxSize - userList.size();
			userList.addAll(service.queryUserList(selectMaxSize));
		}

		return userList;
	}

    在方法体中，方式1和方式2都是可行的。对于简单的循环，尾部的一个判断跳出看上去比一个多行的条件判断更清晰一点。

评论

5 楼 tuti 2010-12-19  

建议楼上，找个同事来看这2种代码。
听听对方的反馈，哪一种更容易理解。

4 楼 zhang_xzhi_xjtu 2010-12-19  

tuti 写道

zhang_xzhi_xjtu 写道

line6是多余的，只是为了保持结构一致才这么写的，个人行为。

既然是讲“重构”，多余的代码就需要清理掉，否则就不是“平铺直叙”。

这个有一定的道理，但是我觉得还是有商量的余地。

int selectSize = size - userList.size(); 

这个和下面的代码块的结构保持一致。从语义上保持一致。

翻译一下三个代码块对当前要查询的用户数的处理
1 为用户数上限数减去已经查找到的用户数。
2 为用户数上限数减去已经查找到的用户数。
3 为用户数上限数减去已经查找到的用户数。

而省掉这个，是建立在这个是第一次查询，已经知道已经查找到的用户数为0。
等于说，
1 为用户数上限数（第一次默认知道已经查找到的用户数为0）。
2 为用户数上限数减去已经查找到的用户数。
3 为用户数上限数减去已经查找到的用户数。

我选择第一种，是因为看上去虽然有一点冗余，但是这个冗余增强了代码含义的一致性。

3 楼 tuti 2010-12-18  

zhang_xzhi_xjtu 写道

line6是多余的，只是为了保持结构一致才这么写的，个人行为。

既然是讲“重构”，多余的代码就需要清理掉，否则就不是“平铺直叙”。

2 楼 zhang_xzhi_xjtu 2010-12-18  

tuti 写道

Line 6, 多余。
返回UserList的方法都设计的不好， 应该不需要判null。

line6是多余的，只是为了保持结构一致才这么写的，个人行为。

get底层的服务，别人未必想的那么周全，如果它的注释中，没有声明一定不返回null的话，我觉得判一下null是比较安全的做法。

1 楼 tuti 2010-12-18  

Line 6, 多余。
返回UserList的方法都设计的不好， 应该不需要判null。