做Python代码重构 竟发现这些潜规则

关键时刻，第一时间送达！

我们的代码库混乱得一塌糊涂。所以有一天，我们决定做一些改进：也就是移动一堆文件。于是，我们花了两个月的时间完成了此次重构。

【编者按】本文作者Craig Silverstein 写了一系列文章来记录可汗学院在 2017 年和 2018 年对 Python 代码进行的重大重构，本文为其中第一篇。在本文中，作者介绍了代码在哪些地方出现问题，想要努力修正的内容，以及这项工作为何如此之困难，从而来帮助人们利用本文中介绍的技巧和代码来避免这些困难。

注：可汗学院(Khan Academy)，是由孟加拉裔美国人萨尔曼·可汗创立的一家教育性非营利组织，主旨在于利用网络视频进行免费授课，现有关于数学、历史、金融、物理、化学、生物、天文学等科目的内容。

以下为正文：

我们的代码库混乱得一塌糊涂。所以有一天，我们决定做一些改进：也就是移动一堆文件。于是，我们花了两个月的时间完成了此次重构。

我们的代码库究竟有什么问题？可能包括：

有一些文件名为 parent.py，coaches/parent.py，users/parent.py 和 api/internal/parent.py，每个文件都包含了有关可汗学院学生家长的部分逻辑。

有一个名为 login.login.Login.login 的方法。 （这个方法是登录过程的一部分。）

根目录有 234 个 python 文件。超过 10%的代码保存在根目录。不看也知道，很多文件之间根本毫无关联。其中有 ip_util.py 等底层工具，也有 dismissed_items.py 等奇怪的特殊工具，还有 activity_summary.py 之类的应用程序逻辑文件，以及 appengine_stats.py 等开发工具。我们还有 93 个 Python 子目录。 （另外还有一些非 Python 代码。）虽然根目录被各种文件塞满，但是其中 30 个子目录仅包含不到 5 个文件。

▌这些问题都是如何发生的？

说的冠冕堂皇一点就是日积月累。大家在创建子目录的时候，以为那个目录会成为重大项目，但是事实却并非如此。而大家把新建的文件放在根目录下的原因是根目录下有个相似的文件。最终问题就一点点积累起来了。在找不到与手头的工作相关的已有代码时，我们一般都会在新文件中写新的代码。而当代码的组织变得毫无道理时（与公司当前的产品或与组织结构不相关时），大家找不到更好的地方保存新文件，所以就随便放了。没有人专门负责代码的结构，代码永远也得不到任何修整与重塑。

▌我们进行了哪些改善？

我们在 2017 年底预留了 6 周时间，让整个公司集中精力解决技术债务，因此我们 3 人决定利用这段时间来重塑代码库（Python 部分）。

注：我们只重构了 Python，因为 Javascript 代码已经组织得很好了。JS 代码组织得越好，用户的下载包就越紧凑，且用户体验越快。因此，JS 代码在提高性能的时候已被清理得很整齐，而 Python 代码还很乱。

我们从子目录着手。一位熟悉大部分代码库的高级工程师设计了初步的目录结构。其中一些用于 test_prep 或 translation 等产品。其他一些则用于 coaches 或 login 等主要数据结构和工作流程。还有一些用于邮件或 pubsub 等底层的基础设施。然后他们花了两天时间将代码库中的每个文件都转入了这些目录中。有几十个无法分类的文件，暂时标记为 TODO。

经过这次粗略的修整后，真正的工作才拉开序幕。另一个组重新浏览了代码库中的每个文件，并根据需要进行重新分类。在这个过程中，有些目录被删除了，有些新的目录加了进来，还有些目录被分割或合并了。经过更仔细的检查后，文件最终被转移到别的目录下。期间经常遇到文件被拆分的情况，因为它们保存了不相关的代码。（尤其是一些较大的文件被分到 4 个不同的目录中！）这项工作没有捷径，我们几乎把代码库中所有代码都看了一遍。

这一步中最有意思的事情是我们如何重新考虑代码库的一些部分。例如，以前我们的 emails 目录保存了可以向用户发送的所有类型的电子邮件。经过此次分析后，我们意识到如果 emails（现在改名为 email）中只包含邮件发送框架的话，代码会更整洁，并且每个具体的邮件应该与其最密切的代码保存在一起（比如 SAT 相关的邮件保存在 sat 中，新用户的邮件保存在 login，等等）。与之类似，我们将 API 处理程序从 api 目录中拿出来，放到各自相关的项目目录中。这两种组织方式各有利弊，但是之前我们想都没想就采用了前者。在重构过程中，我们重新考量了这两种方案。

▌重构工作的难点

代码审查。虽然 slicker 很好用，但是有些模棱两可的情况下，每个变化都必须经过仔细审查。例如，我们将 feeds.py 重命名为 content_render/rss/feeds.py。那么考虑如下代码：

_MODULE_WHITELIST = ["feeds"，…]

user_data ["feeds"] = _get_feeds_for_user（）

对于第一行代码，我们希望把 feeds 改成 content_render.rss.feeds，但不希望变更第二行代码。自动化工具无法掌控这一点，所以我们只能靠代码审查（当然还有测试）。我曾经一整天一整天审查代码。

代码合并冲突。谁都没有想到我们会在解决代码合并冲突上花费如此多的时间。我们预料到了在移动文件的时候，如果有人用到我们的代码库，那么可能出现代码合并冲突，所以我们小心翼翼地与合作伙伴协调，尽量避免这种情况。但是没想到所有的合并冲突实际上都发生在我们内部。我们以为可以同时进行多个文件的重命名，但是后来在合并的时候遇到了大量的冲突。这是因为移动文件时会更改其他文件中的 import 语句。假设 somefile.py 同时 import foo 和 import bar，那么如果一个人重命名 foo.py，而另一个人重命名 bar.py，那么这两个人会同时编辑 somefile.py。第二个完成重命名的人必须合并前一个人的变更。不幸的是，import 语句往往集中在一起，而 git 将这两个更改视为冲突（尽管它们确实不是）。每次这样的冲突都必须手动解决。解决代码合并冲突的时间实际上超过了（有时甚至大幅超出）当初创建提交的时间。

目录结构隐藏的依赖关系。我们有很多的文件包含类似 datafile = os.path.join(__ file__，".."，"testdata"，"whatever")的代码。重构会导致这些代码无法正常运行，而修复这些问题的工作非常枯燥。

Pickle。pickle 库 pickle 类或函数时，它会使用全名：path.to.module.myfunc。如果这个类或函数移动了，那数据就不能 unpickle 了。虽然 pickle 的这个问题不足为奇，但有个特殊的问题是无解的——如果想保存函数名以便后面调用，那么就必须想办法指明函数名。事实上，我们只是需要在 AppEngine 的延迟调用库中这么做，但是 AppEngine 在我们的代码库中用到了很多次。在我们移动文件的时候，这些应用都无法正常工作了。

我们使用了 pickle_util 来解决这个问题。pickle_util 是个 pickle 的 wrapper，在可汗学院代码的重构中，我们利用它来记录符号重命名。当 pickle_util 的 unpickle 发现无法导入的类、函数或其他东西时，它会查找符号表以确定符号的新位置，然后再试着从新位置导入。这样 unpickle 就无需考虑移动文件的问题了。源代码的链接如下，稍加改动就可以用在其他项目上（可以在 pickle 和 cPickle 上使用，但是仅在 python2 上测试过）：

http://engineering.khanacademy.org/supporting-files/pickle_util.py

但是我们遇到一个问题：我们不知道需要用 pickle_util 记录哪些符号，因为我们没有一个列表指明哪些函数或类可能被 pickle 过。虽然可以用 pickle_util 记录代码中的所有符号，但这种做法又笨又慢。所以，我们使用了另一种解决方案：pickle guards。

每次将一个文件移动到新位置时，我们就创建一个 pickle guards 用来“转发”。pickle guard 文件依然保存在旧位置，但它会从新的位置导入文件。这个转发文件永远不会被我们的代码导入（因为所有的引用都参照新位置），但是 pickle 会使用转发文件，因为在 unpickle 符号的时候引用的是旧位置。如果转发文件被导入，我们就会在日志中写入一条消息：“pickle 用到了这个文件的符号！”之后，我们可以查看日志找出需要在 pickle_util 中记录的东西。日志中的内容全部处理完后，我们就可以删除这些 pickle guard 文件，并拥有一个漂亮干净的代码库。

下面是我们重命名时记录的 pickle guard 文件的例子：

google_analytics.py to analytics/google_analytics.py:

logging.error("Should not be importing %s, "

"update pickle_util.py", file)

from analytics.google_analytics import _construct_event_payload # NoQA: F401

from analytics.google_analytics import _fix_payload_unicode # NoQA: F401

from analytics.google_analytics import _send_event_to_ga_sync # NoQA: F401

from analytics.google_analytics import google_analytics_user_id # NoQA: F401

from analytics.google_analytics import mark_ga_activation # NoQA: F401

from analytics.google_analytics import send_event_to_ga # NoQA: F401

我们在移动文件时会创建 pickle guard 文件。这个操作的自动化处理代码链接如下：

http://engineering.khanacademy.org/supporting-files/generate_pickle_guards.py

▌自我反省

我认为，很多事情我们都处理得很好。我们在动手之前很明智地总结了具体的工作内容；即便不用深究代码中的语义，单是移动文件本身的机制就极其错综复杂。

我们没有试图找一天时间禁止任何人修改核心产品，以避免代码合并冲突，并利用这段时间做完所有的工作，这一点还是很明智的。我们曾经考虑过，在专门的时间段内做重大修改虽然看上去很不错，但终归不是个好主意。在重构过程中，出现了很多意想不到的问题，似乎我们需要一个月的时间才能处理完。好在我们从一开始就尽量围绕其他人的日程安排进行了规划，从而避免了与其他开发人员的代码发生合并冲突，同时又不阻碍别人的工作。

但是，现在回想起来，我认为我们可以投入更多的工具，以便自动解决代码合并冲突，那我们就不需要不断修复 import 问题了。这部分工作耗时且容易出错，应该可以用自动化处理。

还有一点，我认为我们生成的 pickle guard 与移动文件的改动应该分开提交。实际工作中，我们同时进行了这两项工作，以致 git 无法识别文件移动，而是认为我们编辑了 google_analytics.py 文件（删掉旧内容，并写入了 pickle-guard 的内容），并创建了一个看似无关的名为 analytics / google_analytics.py 的新文件。现在 git blame 和 git log 已无法正常工作。（我们可以使用 git blame -C，它可以正常工作，但速度很慢。而 git log 的问题目前还没有解决办法。）如果当初我们分开提交，就可以很容易地避免这个问题。

▌两个月后

清理工作完成后，乱糟糟的代码总有可能随时回来。但迄今为止我们还未发现异常情况。代码组织越清晰，新代码的去向位置就越明显，且团队更加容易将代码集中在与项目相关的一两个目录中。当然，随着公司新产品的推出与旧产品的下架，代码结构需要进行调整，但是现在这项工作的难度已经大大降低。而且工作了很长时间的工程师可以更轻松地找到自己的代码。而对于我们这些新员工，只是还没有感受到这些好处。

原文：http://engineering.khanacademy.org/posts/python-refactor-1.htm

作者：Craig Silverstein

译者：弯月

责编：张伟