Resurrecting a Dead Library: Part Three - Rehabilitation

Michael Lynch

复活一个已废弃的代码库:第三篇——重构

原文由 Michael Lynch 发布,订阅该博客

我热爱重构。没有什么比把一团面条式代码梳理开来、让其背后的逻辑以清晰直观的方式呈现更让我满足的了。

我逐渐明白,重构需要格外谨慎。年轻时我更冒失,常常一头扎进遗留代码库,毫无节制地大刀阔斧地改代码。结果往往是几天或几周后才发现,自己删掉了一段看似无关紧要、实则在某个冷门场景下至关重要的细节,把代码给改坏了。

在这篇文章里,我会展示如何谨慎地进行重构。我会讲解自己在一套真实的遗留 Python 库上所采用的重构技巧,包括为减少失误而搭建的开发工具链,以及为固化现有行为而补充单元测试的过程。

这是关于我如何复活ingredient-phrase-tagger的三篇系列文章中的最后一篇,这个库利用机器学习来解析烹饪原料短语(例如“2 cups milk”)并转化为结构化数据。完整背景请参阅第一篇,简而言之,我发现了一个被废弃的库,并让它重新焕发生机,以支撑我的 SaaS 业务:

  • 第一篇:复苏——让代码恢复健康,使其能在任何现代系统上运行
  • 第二篇:稳定——在修复代码的同时防止功能回退
  • 第三篇:重构(本文)——开始重构代码

一只寄居蟹正被从壳中拉出来

我们进展到哪了?

在前两篇文章中,我创建了一个定制的 Docker 镜像,从而可以在任何地方使用这个库,并添加了端到端测试来保障其高层行为。每当代码库发生变更,Travis 持续集成都会构建所有依赖,并在受控环境中执行测试

到目前为止,我还没有真正改动过代码本身。我只是在现有代码之上添加了工具和脚本来验证其行为。现在,有了安全修改代码所需的全套机制,我终于可以开始重构了。

统一空白格式

开发者永远不该在空白格式上浪费精力。每当开启一个新项目,我都会尽早把空白格式自动化。

对于 Python 项目,我通过YAPF(Yet Another Python Formatter)来实现。在这个项目中,我的第一次代码改动就是按照我偏好的标准——Google Python 风格指南——对所有文件重新排版:

yapf \
  --in-place \
  --recursive \
  --style google \
  ./ \
  --exclude="third_party/*" \
  --exclude="build/*"

这次改动带来了大量的代码变动,但我确信这是安全的,因为 YAPF 是一个成熟的工具,而且我的端到端测试依然通过。

我特意将这次拉取请求严格限制为包含空白格式的改动,以免在大量格式噪音中掩盖其他改动,让其他开发者难以审查。

YAPF 改动后的 diff

用 YAPF 修正空白格式后的 diff

为了确保后续改动都能遵守同样的风格约定,我在构建脚本中新增了一项检查:

yapf \
  --diff \
  --recursive \
  --style google \
  ./ \
  --exclude="third_party/*" \
  --exclude="build/*"

这条命令与前面那条基本相同,只是把 --in-place 换成了 --diff。如果 YAPF 检测到空白格式违规,就会打印出差异并返回失败的退出码,从而让构建脚本以失败告终。

加入静态分析

pyflakes 是我每个 Python 工具链中都会加入的另一个实用组件。它通过静态分析来发现诸如未初始化变量或未使用导入之类的粗心错误。

把它加入了 ingredient-phrase-tagger 的构建脚本,它立刻就发现了一个未使用的导入:

$ pyflakes \
    bin/ \
    ingredient_phrase_tagger/
ingredient_phrase_tagger/training/utils.py:3: 'string' imported but unused

是时候读代码了

你可能已经注意到,在整个过程中我一直在避免去真正理解代码。对库的行为,我只保持着非常粗浅的了解就勉强应付了下来。

我发现,阅读代码最好的方法就是边重构边测试。著名软件专家Martin Fowler对此有最精辟的描述:

当我面对不熟悉的代码时,我必须试着去理解它在做什么。我会看上几行,然后对自己说,哦,这段代码原来是在做这件事。通过重构,我不会止步于心里的那个念头。我会真的去改代码,让它更准确地反映我的理解,然后通过再次运行代码来检验我的理解是否正确。

-Martin Fowler,《重构:改善既有代码的设计》

解决糟糕的代码组织

这个库有 80% 的代码都集中在仅仅两个文件里:cli.py(命令行界面)和 utils.py(工具函数)。换句话说,作者把代码分成了两个筐:“用户界面”和“其他所有东西”。但即便是这种划分也不够清晰。

cli.py 里几乎没有多少代码真正与命令行读写相关。它只包含一个名为 Cli 的类,里面有以下几个方法:

  • run
  • generate_data
  • parseNumbers
  • matchUp
  • addPrefixes
  • bestTag
  • _parse_args

我的首要任务是精简 Cli 类,让它成为一个更符合逻辑的命令行界面抽象。

解剖 Cli

要拆分 Cli 类,我需要一个切入点。generate_data 看起来显然不该属于负责管理用户界面的类,但我还不能马上把它移走。generate_data 通过 self 参数调用了 Cli 的其他方法,意味着它与类的其余部分共享状态。

真的是这样吗?cli.py 中的每个函数都是 Cli 类的成员方法,但它们真的共享实例变量吗?

我查看了 Cli 的构造函数:

def __init__(self, argv):
      self.opts = self._parse_args(argv)
      self._upstream_cursor = None

构造函数给 self._upstream_cursor 赋了值,但没有任何地方引用过这个变量。这是死代码,直接删掉就行。

另一个成员变量 self.opts 倒不是死代码,但只有两个方法引用了它:rungenerate_data

既然没有共享状态,Cli 的其他公共方法根本没必要作为方法存在。它们完全可以作为模块级的普通函数存在。更进一步,我可以把它们移到一个更能体现其用途的全新模块中,而不是留在 cli 里。

形成清晰的抽象

一旦发现 Cli 的大多数方法都可以放到另一个模块中,我就得设计这个新模块。当然,我可以把所有函数都搬过去并全部公开,但我更想在 Cli 类与这个新模块之间找到一个最小化的接口。

我意识到,Cli 是在 generate_data 的循环体内部调用其他所有函数的。如果我把那段代码提取成一个新函数,Cli 就只需要访问这个新函数,而不再需要之前那些方法了。

YAPF 改动后的 diff

generate_data 的循环体提取为名为 translate_row 的新函数

这一改动让 Cli 类变得更精简、内聚性也更强。现在它只剩下两个公共方法和一个私有方法:

  • run
  • generate_data
  • _parse_args

它依然不够完美,但已经比之前臃肿的接口好多了。诚然,我还想做更多改动,但那些只能先放一放。

为了尽可能降低出错概率,我在重构时严格控制每个拉取请求的范围。在文件之间移动代码时,尤其要尽量减少改动,因为移动本身就很容易让人忽略行级别的细微修改。

我的端到端测试通过了,这说明这次移动没有破坏什么重要功能,但工作还没完。我的重构产生了一个新函数,这意味着我需要一个新的单元测试来覆盖它。

我的第一个单元测试

创建单元测试很容易。我在 translator.translate_row 的开头和结尾临时添加了调试日志语句来打印输入和输出。这些值就成了我第一个单元测试的输入和预期输出:

def test_translates_row_with_simple_phrase(self):
    row = {
        'index': 162,
        'input': '2 cups flour',
        'name': 'flour',
        'qty': 2.0,
        'range_end': 0.0,
        'unit': 'cup',
        'comment': '',
    }
     self.assertMultiLineEqual("""
2\tI1\tL4\tNoCAP\tNoPAREN\tB-QTY
cups\tI2\tL4\tNoCAP\tNoPAREN\tB-UNIT
flour\tI3\tL4\tNoCAP\tNoPAREN\tB-NAME
""".strip(),
                              translator.translate_row(row).strip())

我当时仍未完全理解这个函数的作用,但这个单元测试让我离理解更近了一步。我看到它处理的是库的训练数据,这些数据存放在一个看起来像这样的 CSV 文件中:

indexinputnameqtyrange_endunitcomment
1622 cups flourflour2.00.0cup

它返回了一组以制表符分隔、供库内机器学习引擎理解的值。

又补充了几个单元测试来覆盖不同类型的原料:带分数的原料("1 1/2 teaspoons salt")和带备注的原料("Half a vanilla bean, split lengthwise, seeds scraped")。

将单元测试集成到构建中

单元测试如果没有集成到构建流程中,就没什么意思,所以我更新了构建脚本来执行它们:

在构建脚本中添加单元测试命令的 diff 截图

在构建脚本中加入单元测试执行命令

由于 Travis 持续集成已经在每次代码变更时都会运行我的构建脚本,我在下一次 Travis 构建中就看到了单元测试的输出:

单元测试日志输出

Travis 构建输出中的单元测试日志

加入代码覆盖率

在重构过程中,我很喜欢看着代码覆盖率随着更多代码被纳入测试而一点点上升。在 Python 项目中,我使用coverage 模块来收集覆盖率信息,并用Coveralls在网页仪表盘上展示结果。

从 Python 原生的单元测试运行器切换到 coverage,只需对构建脚本做一个微小的改动:

-python -m unittest discover
+coverage run -m unittest discover

然后,我在 Travis 配置中添加了 after_success 字段,以便 Travis 将代码覆盖率信息上传到 Coveralls。

after_success:
  - pip install pyyaml coveralls
  - coveralls

我满怀期待地打开 Coveralls 查看覆盖率统计,结果……

Coveralls 未显示任何结果的截图

Coveralls 未显示任何代码覆盖率信息

什么都没有。

我的代码覆盖率去哪了?

我过去在几十个项目中都用过 Coveralls,不明白为什么这次没有显示任何内容。这本来就是一个简单的 Python 项目。coverage 命令应该会生成一个名为 .coverage 的文件来保存覆盖率信息,而 coveralls 命令则应该把它上传到 Coveralls 仪表盘。

哦,原来问题在这里!coverage 命令是在我的 Docker 容器内运行的,而 coveralls 二进制文件却是在标准的 Travis 环境中运行的,所以它找不到 .coverage 文件。我根本没有把它从 Docker 容器复制到外层的 Travis 环境中。

这很好解决。我只需加一条命令把 .coverage 文件从 Docker 容器中提取出来:

after_success:
  - pip install pyyaml coveralls
  - docker cp ingredient-phrase-tagger-container:/app/.coverage ./
  - coveralls

可 Coveralls 仪表盘依然什么都没显示:

Coveralls 再次未显示任何结果的截图

Coveralls 仍然没有显示任何代码覆盖率信息

不过,这次 Travis 构建打印出了之前构建中没有的输出:

$ coveralls
Submitting coverage to coveralls.io...
No source for /app/ingredient_phrase_tagger/__init__.py
No source for /app/ingredient_phrase_tagger/training/__init__.py
No source for /app/ingredient_phrase_tagger/training/cli.py
No source for /app/ingredient_phrase_tagger/training/translator.py
No source for /app/ingredient_phrase_tagger/training/utils.py
Coverage submitted!
Job #177.1
https://coveralls.io/jobs/39259674

这时我才意识到还有另一个问题。

Travis 和 Docker 对文件系统有着冲突的视角。例如,它们各自看到的 cli.py 文件路径是这样的:

环境文件路径
Docker 容器/app/ingredient_phrase_tagger/training/cli.py
Travis/home/travis/ingredient_phrase_tagger/training/cli.py

这样一来,coveralls 在 Travis 中打印的错误信息就说得通了:

No source for /app/ingredient_phrase_tagger/training/cli.py

Coveralls 找不到文件,是因为 .coverage 中的路径是基于 Docker 容器内的文件系统视角的。/app 这个路径在 Travis 文件系统中根本不存在。

我该如何弥合这两个对同一批文件有着不同视角的环境之间的鸿沟呢?我找到了一个办法,但有点绕。

一个迂回的路径转换办法

coverage 的文档中,我注意到它支持一个paths 选项,其中讨论了如何合并来自多个文件系统的路径:

paths 参数文档截图

paths 选项的文档

为了使用这些选项,我创建了如下的 .coveragerc 文件:

[run]
source = ingredient_phrase_tagger

; Run in parallel mode so that coverage can canonicalize the source paths
; regardless of whether it runs locally or within a Docker container.
parallel = True

[paths]
; the first path is the path on the local filesystem
; the second path is the path as it appears within the Docker container
source =
  ingredient_phrase_tagger/
  /app/ingredient_phrase_tagger/

我的新方案是在 Docker 容器内运行 coverage 命令,然后在 Travis 环境中执行coverage combine 功能,它会将所有路径规范化为 Travis 文件系统的路径。

应用这个方案后,我的Travis 配置中的 after_success 部分变成了这样:

after_success:
  - pip install pyyaml coveralls
  # Copy the .coverage.* file from the Docker container to the local filesystem.
  - docker cp ingredient-phrase-tagger-container:/app/$(docker exec -it ingredient-phrase-tagger-container bash -c "ls -a .coverage.*" | tr -d '\r') ./
  # Use coverage combine to canonicalize the source paths.
  - coverage combine
  # Upload coverage information to Coveralls.
  - coveralls

终于有了代码覆盖率

我对完整方案进行了测试。最终,Coveralls 接收到了结果,并展示了我的代码覆盖率数据

显示代码覆盖率统计的 Coveralls 截图

Coveralls 终于显示了代码覆盖率信息。

我宣布,这个库已重获新生

在集成了代码覆盖率跟踪之后,我觉得这个库算是真正活过来了。它还拿不到什么质量奖,但基础设施已经就位,无论是我还是其他开发者,都可以带着很高的信心继续迭代代码了。

在这系列博文中,我讲述了自己如何通过一个个小而离散的步骤来改进这个库。这最大限度地降低了引入缺陷的可能性,但或许也掩盖了整体图景。为了提供一些视角,请允许我回顾一下在复活这个库的过程中所做的高层改进:

之前之后
仅能在 OS X 上构建可在任何支持 Docker 的环境中构建
没有端到端测试拥有完善的端到端测试
没有单元测试拥有少量单元测试,并具备轻松添加更多测试的机制
没有代码覆盖率信息每次提交都会度量代码覆盖率,并长期维护覆盖率历史
没有自动化构建每次提交都会自动构建和测试代码
代码风格不一致通过自动化工具强制执行风格约定
开发者必须手动发现未使用的导入和未初始化的变量通过静态分析自动捕获粗心错误

重构一次,只为丢弃

鉴于我对这些改动如此自豪,你可能会惊讶地发现,在继续改进代码几周后,我最终还是抛弃了它,转而进行了彻底重写。

……计划先丢弃一个版本;反正你最终都会这么做的。

-Fred Brooks,《人月神话:软件工程论文集》

我越是重构代码,就越发认识到其基础架构存在问题。这并不意味着我之前改进代码的努力是白费的——我需要亲手实践才能形成深刻的理解。一旦完全理解了,我就有信心从零开始重写,以获得更好的可维护性和性能。

成果就是一项名为Zestful的服务。它提供了与 ingredient-phrase-tagger 类似的功能,但以托管 API 的形式提供。客户无需经历我为让原库可用而经历的那些繁琐步骤,就能立即解析原料。

如果你想看看 Zestful 的实际效果,可以查看在线演示

Zestful 原料解析演示截图


封面插图由 Loraine Yow 绘制。我复刻的 ingredient-phrase-tagger 库可在GitHub 上找到。我基于该库提供了一项名为Zestful 的托管服务。

本文章由 muse-spark-1.2-contributor 进行翻译

评论