深度解析：清理烂代码

分享到

深度解析：清理烂代码

发布于2013-7-8

摘要：烂代码不一定是问题，只要它们没有出错，没有人会对它嗤之以鼻。但不幸的是，它们没被发现的概率太小了。错误会被发现。需要新的功能，新系统发布了。现在你不得不面对这堆恐怖的代码，试着去清理它们。
猜猜看怎么了！你正”继承“（接收）了一堆混乱的旧代码。恭喜你！现在都是你的了。混乱的代码可能来自任何地方。中间件，网络，可能来自你自己的公司。

你知道在一个角落里有一个家伙，没有人过去管他在做什么。猜猜看他一直在做什么？辛辛苦苦写出了代码，却是一堆烂代码。

你还记得这个模块是一个家伙几年前写的，在他离开公司之前。这个模块已经有20个不同的人加过补丁，进行过代码修复，而且他们也并不理解代码到底是做了什么。是的，就是这样的代码。

或者你从网上下载下的开源的软件，你知道它非常的可怕，但是它解决了一个非常专的并且对你来说非常棘手的问题，解决这个问题你可能要花上几年。

烂代码不一定是问题，只要它们没有出错，没有人会对它嗤之以鼻。但不幸的是，它们没被发现的概率太小了。错误会被发现。需要新的功能，新系统发布了。现在你不得不面对这堆恐怖的代码，试着去清理它们。这篇文章为这种不幸的情况提供了一些建议。

0. 值得清理么？

第一件你需要问问自己的事情就是代码值得清理么。我不是说当问到是否要清理代码时，你一定要回答是或者一定回答不是。是你对代码负有责任，也是你需要一直面对它们直到最终写出的代码是你乐意维护的，也是你很自豪的放入代码库的。

如果你觉得就算代码看起来很可怕，也不值得浪费你本来就很紧张的时间来修复它们。所以你仅仅做了最最微小的调整解救燃眉之急。

换句话说，你也可以将代码看作自己的，也可以看作是别人的。

两种情况都有优缺点。优秀的程序员看到烂代码时会觉得很难受。他们会拿出火把和叉子并且高呼：“太乱了，太乱了”。这是一种优秀的品质。

但是清理代码是一个繁杂的工作。很容易就低估了时间。甚至有时候和从头开始写代码一样的耗时。并且短期并没有带来任何的短期效应。两个星期的时间清理代码并不会带来任何新的功能，但有可能引入一些新的错误。

另一方面，如果长时间不清理代码可能会带来灾难性的毁灭。混乱是代码的杀手。

所以，这并不是一个容易做出的决定。需要考虑一些事情：

1）你期望对这段代码做多少改变？你是希望仅仅修改这个小错误呢，还是这段代码还要使用多次，所以你希望将它“调教”的好些，并且加上新的功能。如果仅仅是修复一个错误，那么最好是别打草惊蛇。然而，如果这个模块你需要长期折腾的话，那么现在开始花点时间来清理它吧，之后会省掉很多烦恼。

2）你需要或者是你想引入上游的更新吗？它是一个正在开发当中的开源项目吗？如果是的话，并且你想做改变的是上游的代码，那么你不能对代码有大的改动否则当你每次pull代码的时候都会经历一场merge的噩梦。所以你需要做一个友好的团队合作者，接受这个错误，将带有你修正的代码补丁发给代码的维护者。

3）要做多少工作？你一天内实际上能清理多少行代码？我们估计多于100行，少于1000行，好，我们假设是1000行。所以如果一个模块有30,000行代码的话，你可能需要一个月的时间。你有那么多时间吗?值得这么做么？

4）它是你核心的功能吗？如果这个模块只是边缘的模块，譬如字体渲染或者图像渲染，你可能并不在意它是否是乱七八糟的。你可能全盘不要，将来用另外的东西来代替，谁知道呢。如果这段代码关乎核心的性能，你需要慎重对待。

5）这段代码有多糟糕？如果代码仅仅有一点点糟糕，那么可能你还是可以忍受的。如果它是不可理喻的，令人崩溃的话，那么我们就必须对它下手了。

1. 建立测试用例

要认真清理一段代码意味着花一段时间来彻底清理它。你可能会毁坏它们。

如果你有一个比较好的测试用例，有一定的覆盖率，你将会很容易知道什么已经损坏了，并且你能够很快的知道你犯了什么愚蠢的错误。想要节省建立测试用例的时间在整个的清理代码的过程中是可笑的。建立测试用例吧。这是你第一件需要做的事情。

单元测试是最好的，但是所有的代码并不适应单元测试。如果单元测试过于繁琐，就换用集成测试吧。譬如，一个游戏关卡中需要一个人物完成一系列的动作和你清理的代码有关。

这样的测试更加耗时，所以不可能在每一次更改之后都测试一次，虽然这是最理想的情况。因为你将每一次改变都放到了版本控制系统中，所以情况还不是那么糟糕。所以每一段时间（比如，五个更改）就测试一次。当你发现了一个问题时，你可以通过二进制搜寻最近的几次commit中找到什么地方导致了问题的发生。

如果你发现了测试没有发现的问题，确保将这个也加入到测试中，以便将来可以测试它。

2. 使用代码版本控制系统

还有人需要被告知要使用代码版本控制系统吗？我希望没有。

清理工作是很关键的。你可能要做很多很多小的修改。如果什么地方出错了，你想回顾版本历史，你可能找到它错在哪。

如果你和我一样，你可能有时重构（清理愚蠢的类）的时候会出错，并且后来意识到这并不是个好的点子，或者这是个好点子，但是如果先做了什么之后所有的一切会变得更简单。所以你想快速的恢复一切到原状并且重新开始。

你的公司应该已经有代码控制系统了，你可以在不同的分支进行修改，在不打扰别人的情况下随意的commit。

就算情况不是这样的，你也应该使用版本控制。下载Mercurial(或Git)，创建新的仓库，将代码从你们公司的愚蠢的系统中签出并放在这里。在库中commit你的更改。当你完成了之后你可以将所有的一切merge到那愚蠢的系统中。

拷贝库到一个代码控制系统中仅仅需要几分钟。很值得这么做。如果你不懂Mercurial，花一个小时学习它。你会为你这么做感到高兴的。如果你愿意的话，花30个小时学习下Git(我是开玩笑的！并不用这么久。现在是“nerd”战斗的时候了！)

3. 每次仅仅做一个小小的改动

有两种方法改进坏的代码：革命和改革。革命是用火把一切都烧掉，从新写一遍。改革是在不破坏的基础上每次只进行一点小小的改变。

这篇文章是关于改革的方法。我不是说革命的方法从来不是必要的。有时代码太糟糕了，需要用革命的方法。但是那些觉得改革的进度太慢的人们往往会鼓励改革，然而经常没有意识到问题的复杂性，并最终并没有比现存的系统更好。

Joel Spolsky写过一篇经典的文章，他没有掉入到这个紧张的争论的陷阱中。

改革的最好的方法就是一次只做一个小的改变，测试它，并且commit它。当一个改变很小时，它更容易理解改动的后果以及确保改动不会影响现有的功能。如果什么地方出错了，你仅仅需要核查很少的一部分代码。

如果你开始做更改并且意识到改得很糟糕，那么你恢复到上一次的commit，不会损失太多的无用功。如果你过了一段时间才发现什么地方有细微的差错，你可以在版本历史中使用二进制搜找到导致问题的更改。

最常见的错误就是一次进行多处更改。譬如，当去除不必要的类层次的势后，你发现API的方法并不是像你喜欢的使用方法，而你打算重新组织它们。不要这么做！先去除层次结构，commit之后再更改API。

聪明的程序员懂得组织，所以他们也不需要太聪明。

试着找一个途径，沿着这个途径你可以把代码变成你想要的模样，每次只有一点点改动。譬如，第一步你重命名方法，使之名字更合理。下一步，你可以将成员变量变成方法的参数。然后将算法变得更清楚些，等等。

如果你开始做更改，并且发现比你原先设想的改变要大，不要害怕又退回去，使用更小的更简单的步骤去完成同样的事情.。

4. 不要同时清理代码和修正代码

这是(3)的结果，但是仍然很重要。

这是一个常见的问题。你开始察看一个模块，是因为你想加入某个新功能。然后你发现这个代码相当的糟糕，所以你开始重新组织它并且加入新的功能。

问题在于清理代码和修正错误是完全不同的目标。当你清理的势后，你想让代码看起来更好，而没有改变它的功能。当你修正错误时，你想改变功能。如果你同时清理代码和改正错误，很难保证清理不会改变什么。

先清理代码，然后再在一个干净的基础上，加入新的功能。

5. 删除你没有使用的功能

清理的时间正比于代码的数量，复杂性和糟糕的程度。

如果代码的功能你目前没有使用，而且在可预见的将来也不会使用，那么就删除它，这会减少你浏览的代码数，降低复杂度（删除不必要的概念和依赖）。你会清理的更快的，而且最后的结果会更简单。

不要留着代码仅仅因为“谁知道呢，你可能某一天需要它”。代码是有代价的 – 它需要被移植，修正错误，被阅读以及被理解。你有更少的代码，就更好。就算在最不可能的情况下，你需要这个旧代码，你也能从代码库中找到它。

6. 删除大部分的注释

烂代码很少会有好的注释。它们通常是这样的：

// Pointless:
    
// Set x to 3
    x = 3;
// Incomprehensible:
    
// Fix for CB (aug)
    pos += vector3(0, -0.007, 0);
// Sowing fear and doubt:
    
// Really we shouldn't be doing this
    t = get_latest_time();
// Downright lying:
    
// p cannot be NULL here
    p->set_speed(0.7);

看看整个代码。如果一个注释对你来说不再有意义，也对你理解代码没什么帮助，那么就删除它。否则你只会浪费你的脑力去理解一堆对你理解代码没帮助的注释。

同样的删除那些已经被注释掉的代码。如果你还需要它的时候，它还在你的代码仓库中。

甚至如果注释是正确而且有用的，记住你还可以重构你的代码。可能当你完成重构后，这些注释不再正确了。这个世界上还没有一个单元测试能够告诉你注释是否已经损坏了。

好代码需要很少的注释因为代码自己已经自说明了而且很容易理解。拥有好名字的变量不需要注释去解释它们的用途。函数如果有好的输入输出，没有特殊情况时是不需要说明的。简单的写得很好的算法在没有注释的情况下也是容易理解的。而断言记录了条件和预测。

大部分情况下，最好的做法是删除所有旧的注释，专注于让代码变得干净和具有可读性，然后再在需要的地方添加代码 – 这些注释反应新的API的用途以及你对代码的理解。

7. 避免共享的可更改的状态

共享的可更改的状态是理解代码的最大阻碍，因为它允许隔一段距离的行动，一段代码可以改变另一段完全不同的代码的行为。人们常说多线程是困难的。事实上，是由于线程共享了可更改的状态，才导致了问题。如果你能避免它们的话，多线程并不复杂。

如果你的目标是写高性能的软件，你应该不能避免一切可更改的状态，但是你的代码仍然可以从减少它而获益。为了“大部分功能完善”而努力吧，确保你确切的知道什么状态在什么地方改变了，并且知道原因。

共享的可更改的状态来自不同的地方：

1）全局变量。最经典的例子。现在每个人都知道全局变量的坏处。但是要注意（有时人们会忘记），全局变量是唯一的会造成问题的共享的可更改状态。全局常量并不糟糕，Sprintf也不糟糕。

2）对象 – 装有乐趣的大袋子。对象能够集合很多方法，无疑可以共享很多可变的状态(成员)。如果一个懒惰的程序员需要将一些信息在方法之间传递的话，她可以建立一个新成员，所以可以依照需要来读它和写它。这非常像全局变量。多么有意思！当一个对象有越来越多的成员时，问题就越来越严重。

3）巨大的函数。你可能已经听说它们了。这种神秘的产物栖息在最黑暗的代码洞穴的最底层。心眼坏的程序员在阴暗的酒吧里谈论它们，他们的理智被他们遇见的代码摧毁了：“我不停地向下翻向下翻，我不能相信自己的眼睛。居然有12,000行。”当函数足够长的时候，它们本地变量将和全局变量一样糟糕。我们不可能知道改变2000行之后的一个局部变量会有什么效果。

这里有一些避免共享的可更改的状态的建议：

1）将较大的函数切分成较小的函数。

2）将较大的对象切分成较小的变量，将相关的成员放在一起。

3）将成员变成private。

4）将函数声明const，返回结果，而不是可更改的状态。

5）将函数声明static，从参数获得值，而不是从共享状态那里取值。

6）避免完全使用对象，实现纯净的功能，不要引入副作用。

7）将本地变量声明const。

8）将指针和引用声明const。

8. 避免不必要的复杂性

不必要的复杂性通常是过度工程化的结果 – 支持的结构（如序列化，引用计数器，虚拟接口，抽象工厂，访问者等等）会拖慢真正有实际功能的代码。

有时候过工程化是因为一些项目开始的时候有一些更大的野心，多于实际完成的。更多的情况，我想是因为程序员读了关于设计模式的书之后和瀑布模型之后的想法，他认为过工程化会形成更“坚固”和“高质量”的产品。