重构遗留程序的一次案例学习

　　遗留代码经常是腐臭的，每个优秀的开发者都想把它重构。而进行重构的一个理想的先决条件是，它应该包含一组单元测试用例，以避免产生回归缺陷。但是为遗留代码编写单元测试可不是件容易的事，因为它经常是一团糟。要想为遗留代码编写有效的单元测试，你大概得先把它重构一下。但要重构它，你又需要单元测试来确保你没有破坏任何功能。这种状况相当于要回答是先有鸡还是先有蛋。这篇文章通过分享一个我曾参与过的真实案例，描述了一种可以安全地重构遗留代码的方法。

　　问题描述

　　在这篇文章中，我将用一个真实案例来描述测试与重构遗留系统的有效实践。这个例子的代码由Java编写，不过这个实践对其它语言也是适用的。我将原始场景稍做了些改动以免误伤无辜，并稍做简化以便让它更容易被理解。这篇文章所介绍的实践帮助我重构了近期我所参与的一个遗留系统。

　　这篇文章并不打算介绍单元测试与重构的基本技巧。你可以通过阅读相关书籍以学习该主题的更多内容，如Martin Fowler的《重构：改善既有代码的设计》及Joshua Kerievsky的《重构与模式》。相对而言，这篇文章的内容将描述一些真实场景中的复杂性，我也希望它能够为解决这些复杂性提供一些有用的实践。

　　在这个案例中我将描述一个虚构的资源管理系统，其中资源指的是可指派给其任务的某个人。可以为某个资源指派一个HR票据(ticket)或者IT票据，也可以为某个资源指派一个HR请求或IT请求。资源经理可以记录某个资源处理某项任务的预计时间，而资源本身可以记录他们在某个票据或请求上工作的实际时间。

　　可以用饼图的方式表示资源的使用情况，图中同时显示了预计时间与实际花费的时间。

　　相关厂商内容

　　BPMS基于RDF/OWL快速元数据仓储，重用流程开发周期所有资产

　　QCon上海2013精彩回顾：GitHub Peter Bell："首先，杀死所有产品Owner"

　　QCon上海2013精彩回顾：Twitter工程VP Raffi："分解Twitter"

　　QCon北京2014大会正式启动，面向社会征集演讲话题

　　QCon上海2013精彩回顾：LinkedIn Sam Shah："如何将数据变为产品"

　　相关赞助商

　　QCon全球软件开发大会(北京站)2014，4月25-27日，诚邀莅临。

　　好像不太复杂嘛?不过，真实的系统能够为资源分配多种类型的任务，当然从技术上讲这也不是多么复杂的设计。但当我初次看到系统的代码时，我感觉自己似乎看到了一件老古董，从中看得出代码是如何从开始逐步进化的(或者不如说是退化的)。在一开始，这一系统仅能用来处理请求，之后才加入了处理票据以及其它类型任务的功能。某位工程师开始编写代码以处理请求：首先从数据库中获取数据，随后按照饼图的方式显示数据。他甚至没有考虑过要将信息组织为合适的对象：

　　class ResourceBreakdownService {

　　public Map search (Session context) throws SearchException{

　　//omitted twenty or so lines of code to pull search criteria out of context

　　and verify them, such as the below:

　　if(resourceIds==null || resourceIds.size ()==0){

　　throw new SearchException(“Resource list is not provided”);

　　}

　　if(resourceId!=null || resourceIds.size()>0){

　　resourceObjs=resourceDAO.getResourceByIds(resourceIds);

　　}

　　//get workload for all requests

　　Map requestBreakDown=getResourceRequestsLoadBreakdown (resourceObjs,startDate,

　　finishDate);

　　return requestBreakDown;

　　}

　　}

　　我相信你肯定被这段代码里的坏味道吓到了吧?比方说，你大概很快就会发现search并不是一个有意义的名称，还有应该使用Apache Commons类库中的CollectionUtils.isEmpty()方法来检测一个集合，此外你大概也会疑惑该方法返回的Map对象到底包含了些什么?

　　别着急，坏味道陆续有来。接下来的一位工程师继承了先人的衣钵，按照相同的方式对票据进行了处理，以下就是修改后的代码：

　　// get workload for all tickets

　　Map ticketBreakdown =getResourceRequestsLoadBreakdown(resourceObjs,startDate,

　　finishDate,ticketSeverity);

　　Map result=new HashMap();

　　for(Iterator i = resourceObjs.iterator(); i.hasNext();) {

　　Resource resource=(Resource)i.next();

　　Map requestBreakdown2=(Map)requestBreakdown.get(resource);

　　List ticketBreakdown2=(List)ticketBreakdown.get(resource);

　　Map resourceWorkloadBreakdown=combineRequestAndTicket(requestBreakdown2,

　　ticketBreakdown2);

　　result.put(resource,resourceWorkloadBreakdown)

　　}

　　return result;

　　先不管那糟糕的命名、失衡的代码结构以及其它任何代码美观度上的问题了。这段代码中最坏的味道就是它返回的Map对象了，这个Map对象完全是个黑洞，里面塞满了各种数据，但又不会提示你里面究竟包含的是什么。我只能编写了一些调试代码，将Map中的内容循环打印出来后，才看懂了它的数据结构。

　　在这个示例中，{} 代表一个Map，=> 代表健值映射，而[] 代表一个集合：

　　{resource with id 30000=> [

　　SummaryOfActualWorkloadForRequestType,

原文转自：http://www.infoq.com/cn/articles/refactoring-legacy-applications