重构加重写保证版本功能的空中加油

在一个产品长期的研发过程中,必须时刻对代码保持警惕,一旦发现代码有腐烂的迹象,就需要考虑及时重构,剔除代码的坏味道,让代码焕然一新。然而,在进度的逼迫下,我们承受了及时交付功能的压力,团队成员对糟糕代码的敏感度又不够高,在这二者的夹击之下,稍有疏忽,整个代码库就有可能变得积重难返。

多数时候,我们疲于应对各种需求。一方面,我们发布的版本正在支撑着客户的生产应用;另一方面,我们还需要不断地开发新功能,以满足客户不断提出的新需求或者需求变更。同时,我们对产品有着雄心勃勃的计划,希望它能够在不断的演化下变得更加强大。我们清醒地感知到,若任由代码如此发展下去,代码腐烂的结果会进而导致架构的腐烂。就像一架正在执行飞行任务的航空器,明知到缺少足够的燃料,需要返回基地加油,却又不得不继续飞行,否则无法按时到达目的地。这时,我们就需要针对代码库进行“空中加油”。

理想的方式是对现有的代码库进行小步重构,然而众多阻力让我们没有信心驾驭重构。这些阻力包括:

  • 随着功能的增加,代码库开始变得庞大
  • 代码的腐烂情况较为严重,需要在结构上做重大调整
  • 团队成员欠缺对大型代码库的重构能力
  • 单元测试与集成测试的测试覆盖率太低
  • 重构与新功能开发同时进行,破坏原有功能的风险太大

显然,想要通过重构实现空中加油,谈何容易!但为了避免未来架构的腐化,对现有代码库进行手术又势在必行。因此,基于当前代码库现状、产品需求与团队成员能力,我确定了“重构+重写”的改造策略,美其名曰“空中加油”。

代码的坏味道

要解决的代码问题很多,这里仅以其中一个关键部分作为示例。在我们的产品中,ElasticSearch作为存储主题区数据的数据库,但作为产品,我们还需要应对不同客户的需求,例如针对数据规模相对较小的客户,亦有可能使用关系型数据库,例如Oracle来存储主题区数据。因此在对主题区的数据进行建模时,我们并没有利用ElasticSearch的存储特性,形成主题区业务模型与数据模型的一一对应。主题区业务模型是一个树模型,但在持久化到数据库中时,却是将业务模型拍平,形成关系数据库的表结构。

在确定持久化架构时,一开始我就确定了两个职责的分离,并以gateway和repository作为承担不同职责的对象。gateway负责访问主题区数据库,完成对数据的CRUD操作,而repository则借鉴了DDD的资源库概念,体现的是业务角度的资源存取。

我犯下的一个错误是没有及时进行代码走查,在确定了此架构原则后,由于进度压力与开发能力的问题,团队成员在开发时并没有体会到这种职责分离的本质,不断地编写代码,而实现却与当初确定的原则渐行渐远。等到我发现时,问题已经变得较为严重了:

  • 部分Repository没有守住边界,为了满足自己的业务,越俎代庖做了gateway应该做的事情
  • repository没有按照业务去定义,出现了许多从命名上大同小异的repository,每个开发人员似乎只熟悉或相信自己定义的repository
  • 有时候为了重用,建立了许多不必要的抽象,继承层次变得混乱

而这一切在引入两层主题区(为了实现多数据源处理等功能,我们引入了前置主题区和细节主题区)之后,就变得更加严重了。两个不同的主题区数据结构几乎一致,但访问的ElasticSearch集群以及index和type却又不同。有的开发人员为了方便,例如他要开发的功能仅需要前置主题区,就定义了一个前置主题区专用的repository,从而引入各种混淆的repository。随着需求的演化,最早定义在ElasticSearchGateway中的基本方法已经不能满足需求,于是又不断增加。但这些方法的操作目标大同小异,就出现了许多混淆不清的方法定义,例如查询单条数据、查询多条数据、按照条件查询、查询时执行聚合与排序以及按照范围进行查询,使得gateway的接口方法变得越来越乱,并被各个repository依赖。

需求与版本演化

在2018年12月底,经过不断的测试和bug修复,我们按照客户的进度要求交付了一个相对完整的版本,满足了客户的需求,并已部署到生产环境中投入使用。在投入使用之后,客户的需求又源源不断继续涌来,使得我们发布的版本始终不能保证稳定。按照产品的规划,我们还需要实现一些重要的功能特性,并可能在未来支持更多的客户。不同客户的数据协议、数据源支撑都不相同,这意味着许多相同的功能需要为不同的客户提供各自专有的实现。

然而,代码的重构又变得刻不容缓。一方面我们要开发新功能,另一方面又需要对原有代码进行重构,而这些变更的发布又需要按照计划部署到生产环境。如前所述,由于众多阻力,重构的风险太大。该怎么办?我们当然可以创建版本分支,例如为当前运行的版本建立一个release分支,然后在master上进行重构和新功能开发。问题在于,重构与新功能发布的频率并不一致,后者甚至可能要求每周发布。如果新功能开发和重构都工作在master上,在发布新功能到生产环境时,我们担心重构会引入未知的bug,破坏已经交付的稳定功能。如果新功能在分支的release版本下开发,又因为重构的影响巨大,导致代码无法及时合并,或者修复代码冲突的代价太大。

重构加重写的策略

为了解决这一问题,我们决定仍然将master同时作为开发与重构的版本分支,之前的release版本则作为特殊情况下的后备版本。针对代码的改造,我们则采用重构+重写的策略。

当我们要重构一个类时,尤其是要重构该类的方法时,往往需要事先确定待重构的方法究竟有多少调用依赖。一旦该方法被多个类调用时,重构接口就成了一件非常棘手的工作。即使保留接口不变,仅仅是重构方法内部的实现,也需要慎之又慎,毕竟不同的调用者可能对这个方法实现会有不同的需求,如果代码编写不够清晰,极有可能在重构时不小心破坏功能,引入Bug。

这种判断依赖的方式是由内自外的方式,而我们的策略则反其道而行之,先“守”住外部的调用点不变,然后以重写的方式去替换这个调用。在重写的时候,我们会站在调用者的角度,逐步地完成重写,以求小步地完成替换。每替换一小块功能,都编写测试去覆盖所有分支,同时采用手动测试的方式,在类生产环境下运行,以确保这次小范围的替换没有引入问题。

整个重构加重写的过程如下所示:

  1. 从外部调用者发现它依赖的类
  2. 创建新的类,然后仅将当前外部调用者需要调用的方法原封不动地搬移到新类中
  3. 在调用者内部的调用点,将旧类替换为新类,并保证功能正确
  4. 编写对应的测试覆盖该功能,然后对新类进行重构,运行测试保证重构没有引入错误
  5. 若新类的内部亦依赖了有待重构的旧有实现,则将新类视为当前的外部调用者,重复第1步
  6. 以此类推,直到旧有的类没有别的依赖,则安全删除旧有类

案例:重构AircraftRepository和ElasticSearchGateway

以我们现在的项目为例。ElasticSearchGateway本是需要重构的目标,但它的依赖非常多,如下图所示,一共有50处依赖:

它的其中一个方法queryByCondition()有32处依赖,如下图所示:

该方法是我们需要修改的目标。现在,我们反其道而行之,在外部定位一个面向某个业务发起调用的类,如针对航空器位置业务的一个类AircraftProcessor,它的调用关系如下所示:

图中标记为灰色的类,就是我本希望重构的类,然而根据前面的分析,它们都有多处调用者,要进行重构,就可能牵一发动全身,要做到改变现有代码的结构而不破坏其功能,就好比做一台精密的脑颅手术一般,难度非常大。为此,我们选择的做法是定义一个新的AircraftRepository类,并站在调用者AircraftLocationPreFilter的角度,将它需要的方法原封不动地拷贝到新类中,并在调用者内部以新类替换对旧类的调用。如果已有自动化测试覆盖这一路径,则运行测试,看这一替换是否影响了原有的功能实现。如果没有自动化测试,则需要编写新的测试去覆盖它。可以考虑编写单元测试和集成测试。

针对新类中的实现,可以在不改变其功能实现的前提下,对其进行重构。这个重构包括调整原有的方法接口,也包括对内部实现进行代码质量改进。由于新类只有这一个依赖点,重构产生的影响就会被限制到一个很小的范围。

例如调用者AircraftLocationPreFilter调用了queryDistinctOrgStationBy()方法,它的定义为:

public List<FlightPath.Track> queryDistinctOrgStationBy(String craftNo, String scopeField, String gt, String lt, String aggField, String sortField) {}

AircraftLocationPreFilter对该方法的调用如下所示:

private List<FlightPath.Track> getPeriodTimeTracks(String craftNo, Date startTime, Date endTime) {
return aircraftRepository.queryDistinctOrgStationBy(
craftNo, UPDATE_TIME_FIELD,
DateUtil.transformTime(startTime, DateUtil.YYYY_MM_DD_T_HH_MM_SS),
DateUtil.transformTime(endTime, DateUtil.YYYY_MM_DD_T_HH_MM_SS),
ORG_STATION_AGG_FIELD, ORIGINAL_TIMESTAMP_SORT_FIELD);
}

这个方法的定义存在问题。一方面它的方法名未能清晰表达查询航空器路径的意图,另一方面,方法签名暴露了太多不必要的字段,例如方法的第2、5、6三个参数就应该封装到AircraftRepository中,而对第3、4两个参数的转换逻辑也不应该暴露出来。现在可以对新AircraftRepository的方法自由地进行重构,例如通过“修改方法签名”的重构手法,将方法签名修改为:

public List<FlightPath.Track> queryTracksBy(String craftNo, Date startTime, Date endTime) {}

方法签名的修改会直接影响对该方法的调用,调用代码修改为:

private List<FlightPath.Track> getPeriodTimeTracks(String craftNo, Date startTime, Date endTime) {
return aircraftRepository.queryTracksBy(craftNo, startTime, endTime);
}

现在,针对AircraftProcessorAircraftLocationPreFilter进行自动化与手动测试,保证修改的代码分支没有受到任何影响。

重写、替换、然后重构,这就是主要的三个步骤,如下图所示:

黄色的AircraftRepository是新建的类,它的代码仅为调用者AircraftLocationPreFilter提供服务,因此重构它的代码并没有太大压力。注意,这里的一个要点是仅拷贝AircraftLocationPreFilter需要调用的旧AircraftRepository方法,而非原封不动地将整个旧AircraftRepository拷贝到新类中,因此新类的代码数量以及依赖点都非常的少。我们能够清晰地知道这个改动影响到了哪些代码分支,就通过测试去保护这些分支,使得这一步骤是充满信心的,保证不会引入新的Bug。一旦确认了新类的方法没有任何问题,就可以找到原方法的其他依赖者,采用同样的方式进行替换。在此过程中,旧AircraftRepository不受任何影响,仍然保留着原貌。除非随着替换过程的推进,这个旧类的所有调用者都转向了新类,它才正式退出历史舞台。

AircraftRepository类的方法与实现虽然进行了重构,但它对ElasticSearchGateway的依赖仍然未变。现在可以如法炮制,针对AircraftRepositoryElasticSearchGateway的调用,做相似的重构加重写,如下图所示:

AircraftRepository仅用到了旧ElasticSearchGateway的一个queryByScopeAndTerm()方法

class AircraftRepository...

private List<AircraftLocation> queryLocations(String craftNo, String scopeField, String min, String max)...
List<SearchConsequence> consequences = gateway
.queryByScopeAndTerm(condition, ORG_STATION_AGG_FIELD, ORIGINAL_TIMESTAMP_SORT_FIELD, scopeField, min, max, AIRCRAFT_LOCATION_TABLE);

为之创建的新ElasticSearchGateway也就只包含queryByScopeAndTerm方法。当新ElasticSearchGateway替换了旧类后,重复之前的步骤,再次针对AircraftProcessorAircraftLocationPreFilter进行自动化与手动测试,保证修改的代码分支没有受到任何影响。同理,新类的代码数量和依赖点都非常少,再对新类进行重构就变得更加简单。我们认为旧类的queryByScopeAndTerm()方法既无法清晰地表达意图,也不利于应对各种查询产生的变化。这时,我们引入了Builder模式,修改原有接口为DSL风格的接口。在新的AircraftRepository类中,保持了原来的调用方式:

private List<AircraftLocation> queryLocations(String craftNo, String scopeField, String min, String max) {
Map<String, Object> condition = new HashMap<>();
condition.put(CRAFT_NO_FIELD, craftNo);
QueryResult queryResult = gateway.queryByScopeAndTerm(condition, ORG_STATION_AGG_FIELD, ORIGINAL_TIMESTAMP_SORT_FIELD, scopeField, min, max, AIRCRAFT_LOCATION_TABLE);
return queryResult.all(AircraftLocation.class);
}

重构了新ElasticSearchGatewayqueryByScopeAndTerm()方法之后,调用代码变成了:

private List<AircraftLocation> queryLocations(String craftNo, String scopeField, String min, String max) {
QueryResult queryResult = gateway.query()
.from(AIRCRAFT_LOCATION_TABLE)
.and(CRAFT_NO_FIELD, craftNo)
.range(scopeField, min, max)
.aggregateBy(ORG_STATION_AGG_FIELD)
.orderBy(ORIGINAL_TIMESTAMP_SORT_FIELD)
.run();
return queryResult.all(AircraftLocation.class);
}

通过这样的重构加重写方式,我们新引入的类既在设计上通过重构得到了改进,还能借助这种小步前行的逐步替换方式,确保新实现在业务场景中得到了验证,规避了引入新bug的风险。一旦新接口得到了质量保证,就可以逐渐地替换旧有实现,直到旧有的repository与gateway类不再有任何依赖时,就可以将它们删除,换来更加整洁的代码了。

抽象分支

事实上,这种重构加重写方式与“抽象分支(Branch By Abstraction)”实践不谋而合。在Martin Fowler讨论抽象分支的文章中,他详细地讲述了这个过程。他将我们要替换的类或者模块称之为Flawed Supplier,抽象分支的做法是先为Flawed Supplier建立一层抽象层,并将客户端的调用代码指向该抽象层,同时为其创建单元测试:

接下来提供一个新的supplier,它同样实现了这个抽象层。一旦这个新的supplier实现完毕,就可以逐步修改客户端代码,转而使用这个新的supplier。最后,在验证了功能没有问题,也没有任何客户端代码还需要旧有supplier后,就可以去掉它:

重构加重写策略中新增的类就是这个新Supplier。但我并没有引入一个额外的抽象层,也未创建一个全新的Supplier,而是采用拷贝的方式直接重用代码,以避免引入不必要的bug。可以认为新增的类其实是要替换类的子集,保留了相同的接口和实现,从而避免多余的实现与依赖,使得重构可以更好地进行。

结论

重构加重写的策略虽然慢,但每一步前进的步伐都非常稳健,充分利用了代码量与依赖点少的新类来降低重构的难度。每完成一个新类的重构,我们都需要测试去验证。如果之前没有测试保障,则要求为新实现编写测试去覆盖,相当于在这个修改过程中慢慢地偿还了过去欠下的技术债务。由于旧类没有受到任何影响,即使重构或重写失败,我们还能够安全地返航。

在这个过程中,会有很长一段时间存在新旧共存的状态。在执行重构的过程中,如果别的团队成员正在开发的新功能需要调用被重构的接口,由于重构还没有完成或未通过全面的测试,则允许该新功能继续调用旧有的类,保证了新功能的开发不受影响。倘若替换已经完成,旧类不再有存在价值时,则需要及时果断地将其删除,新开发的功能也要立即转为对新类的调用。由于新类的功能已经得到了保证,不必担心调用它会引入错误。

执行重构加重写的过程需要小步前行,并及时提交新增或重构的代码,同时提高自动化测试的覆盖率。所有工作都在一个版本上进行,并保证重构加重写的功能都是正确可用的,保证该工作版本随时处于可上线的状态。

在进行重构加重写时,还允许灵活调整人力资源。倘若需求变更或新需求开发的优先级高,交付压力大,就可以只安排少数人员专注于重构加重写,其他人员全力以赴新功能的开发,甚至在压力太大的情况下,停止进行重构加重写。反之,若开发压力较小,又可以匀出更多的人来进行重构加重写,尽快还清技术债。显然,这种策略使得我们进可攻退可守,理想状态下,甚至能够在一直保证不败的前提下,拥有随时发起进攻的选择权。