论坛首页 Java企业应用论坛

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

浏览 2205 次
精华帖 (0) :: 良好帖 (0) :: 新手帖 (0) :: 隐藏帖 (0)
作者 正文
   发表时间:2010-12-13   最后修改:2011-06-06
很多应用程序的主要目的就是用计算机来代替人处理真实世界中的问题,从而极大的提升处理问题的能力和效率。但是,真实世界和计算机世界之间有着天然巨大的差异。软件开发领域一直以来都有一个重要的发展方向,提供一些平台,方法论,尽量减少这个差异。面向对象语言,面向领域编程,极限编程等等,为解决该问题在不同层次提供了解决方案。
作为程序员,在代码级别,也可以通过种种努力减少这个差异。
先看下面一段代码,需求和场景是这样的。
传入一个整数,指定一次查询返回用户数的上限,结果返回查询到的用户列表。有3个不同的数据源可以用来查询用户信息,这3个数据源在查询的时候是有优先级的。即先查询第1个数据源,如果得到的用户数少于上限,则查询第2个数据源,如果还是少于上限,则查询第3个数据源。
	public List<User> findUserList_0(int limitSize) {

		List<User> userList;
		int selectSize = 0;
		userList = findUserListFromSource1(limitSize);

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

				if (null != userList2 && !userList2.isEmpty()) {
					userList.addAll(userList2);
					if (userList.size() < limitSize) {
						selectSize = limitSize - userList.size();
						List<User> userList3 = findUserListFromSource3(selectSize);
						if (null != userList3 && !userList3.isEmpty()) {
							userList.addAll(userList3);
						}
					}
				}
			}
		} else {
			userList = findUserListFromSource2(limitSize);
			if (userList.size() < limitSize) {
				selectSize = limitSize - userList.size();
				List<User> userList4 = findUserListFromSource3(selectSize);
				if (null != userList4 && !userList4.isEmpty()) {
					userList.addAll(userList4);
				}
			}
		}

		return userList;
	}

如果你耐心的看完了这段代码,我只能说2个字,佩服!当一眼扫到5层的if嵌套,我已经是半晕状态了。我承认,我没有耐心看完这段代码。
该方法的需求和场景都是比较简单的,因此应该存在简单的写法。
重写的代码如下:
	public List<User> findUserList_1(final int limitSize) {

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

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

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

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

		return userList;
	}

这里首先对参数加上final修饰符,明确表明方法中不会修改该参数。
其次,去掉了if嵌套,用相似的代码结构试图更清晰直白的表明这段代码的意图。
虽然代码重构后有所进步,但是,这段代码还存在以下一些问题,可以持续改进。
1 如果可以预期该方法返回的用户列表大小的话,可以设置初始的列表大小。当然,如果应用的场景对于空间,性能的要求不高,从可读性上而言,不传入该参数比较好。
2 如果遵循方法的返回类型为列表时,结果为空时返回空列表而不是null的话,可以使代码更清晰紧凑。当然,这个需要修改底层的API接口,审查调用该API的所有方法。
	public List<User> findUserList_2(final int limitSize) {

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

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

		selectSize = limitSize - userList.size();
		userList.addAll(findUserListFromSource2(selectSize));
		if (userList.size() >= limitSize) {
			return userList;
		}

		selectSize = limitSize - userList.size();
		userList.addAll(findUserListFromSource3(selectSize));

		return userList;
	}

这样看上去好多了。这里遵循的原则就是平铺直叙的用编程语言直接翻译需求,试图用接近自然语言的表达方式来实现需求,从而减少两个世界之间的差异。
还有一种方式是先用自然语言(或者伪代码)描述需求的实现,然后翻译成真正的代码。
用自然语言描述这个需求的实现如下:
1 新建一个空的用户列表。
2 计算下一次查询的上限。
3 到数据源1去查询。
4 添加结果到用户列表。
5 如果用户列表达到上限则返回。
6 计算下一次查询的上限。
7 到数据源2去查询。
8添加结果到用户列表。
9如果用户列表达到上限则返回。
10计算下一次查询的上限。
11到数据源3去查询。
12添加结果到用户列表。
13 返回。
这个实现的自然语言描述和真实代码的实现很像,程序员参照这个自然语言的实现,翻译成对应的代码实现相对比较容易一点。
当然,计算机编程是复杂的,这里的实现还是有一个潜在的问题,代码中有重复的结构。鉴于这里的实现已经比较简单了,所以没有进一步去掉这个重复的结构。但是,如果需求改变,需要查询更多的数据源,或者要查询的数据源可配置的话,代码可以做如下演进。
定义一个接口查询用户列表。
public interface UserQueryService {
	/**
	 * 查询用户列表。
	 * 
	 * @param limitSize
	 *            查询结果的上限数。
	 * @return 用户列表。
	 * */
	public List<User> queryUserList(int limitSize);
}

不同的查询有不同的实现。
	class UserQueryServiceImpl_1 implements UserQueryService {

		@Override
		public List<User> queryUserList(int limitSize) {
			return findUserListFromSource1(limitSize);
		}
	}

	class UserQueryServiceImpl_2 implements UserQueryService {

		@Override
		public List<User> queryUserList(int limitSize) {
			return findUserListFromSource2(limitSize);
		}
	}

	class UserQueryServiceImpl_3 implements UserQueryService {

		@Override
		public List<User> queryUserList(int limitSize) {
			return findUserListFromSource3(limitSize);
		}
	}

原有功能的实现。
	public List<User> findUserList_3(final int limitSize) {

		// 这里的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>(limitSize);

		// 方式1
		for (UserQueryService service : userQueryServices) {
			int temLimitSize = limitSize - userList.size();
			userList.addAll(service.queryUserList(temLimitSize));
			if (userList.size() >= limitSize) {
				break;
			}
		}

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

		return userList;
	}

在方法体中,方式1和方式2都是可行的。对于简单的循环,尾部的一个判断跳出看上去比一个多行的条件判断更清晰一点。
在这个例子中,经过一步一步的演进,一块比较混乱的代码,竟然重构出来一个比较简单的用户查询的框架。这正是软件开发的乐趣所在。
   发表时间:2010-12-18  
Line 6, 多余。
返回UserList的方法都设计的不好, 应该不需要判null。
0 请登录后投票
   发表时间:2010-12-18  
tuti 写道
Line 6, 多余。
返回UserList的方法都设计的不好, 应该不需要判null。


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

get底层的服务,别人未必想的那么周全,如果它的注释中,没有声明一定不返回null的话,我觉得判一下null是比较安全的做法。
0 请登录后投票
   发表时间:2010-12-18  
zhang_xzhi_xjtu 写道

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


既然是讲“重构”,多余的代码就需要清理掉,否则就不是“平铺直叙”。
0 请登录后投票
   发表时间:2010-12-19  
tuti 写道
zhang_xzhi_xjtu 写道

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


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


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

int selectSize = size - userList.size(); 


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

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

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

我选择第一种,是因为看上去虽然有一点冗余,但是这个冗余增强了代码含义的一致性。
0 请登录后投票
   发表时间:2010-12-19  
建议楼上,找个同事来看这2种代码。
听听对方的反馈,哪一种更容易理解。
0 请登录后投票
论坛首页 Java企业应用版

跳转论坛:
Global site tag (gtag.js) - Google Analytics