我见过的最糟糕代码

作者 | Mehdi Zed

译者 | 王强

策划 | 万佳

在本文，我将向你展示我见过的一些最糟糕的代码。除非你希望被同事和用户讨厌，否则这些“魔鬼”永远都不应该被放到人间。我们会发现，通过一些好的实践，其实很容易避免它们。

1“魔鬼代码”

需要改进的代码与所谓的“魔鬼代码”是不一样的。

不管使用的是哪种语言，“魔鬼代码”都是糟糕的代码，因为它会危及项目的稳定性和可维护性。告诉你，我见过很多“魔鬼代码”。

当它堆积如山时，你的项目很快就会变成十八层地狱的样貌。如果你喜欢到处捅娄子，那么团队领导看你的眼光也会越来越不一样。

2模棱两可和前后矛盾

很久很久以前，在一个遥远的星系中，我在一个清晨醒来，被世界末日般的景象吓得跳了一跳。

生产环境出了一个了不得的错误。生产中的所有系统票证莫名其妙地返回“null”。到处都乱成一团。所有人都像无头苍蝇一样到处乱跑。

我百米冲刺到我的工作站，第一个条件反射就是看 Kibana。没有日志，什么都没有。完蛋了，这可不是什么好的开始。

因此，我决定追溯票证的创建路径。

为此，我必须深入研究系统自盘古开天辟地以来创建的那些内部库。考古工作结束的时候，我找到了问题来源之一的一堆文件。

然后，我看到了这样的景象：

// use id and expire to get ticketasync function get_ticket(i, expire) {  return CheckisNotExp(expire).then(async function() {    var t = await GetTicketModel(i)    if (t) {      return t    } else {      logger.error(JSON.stringify(t))      return null    }  }).catch(async function(e)  {      logger.error(JSON.stringify(e))      return null})}

真的有一个“i”变量吗？我们现在在哪？这是一个 id，对不对？它是整数还是 UUID？到底是什么到期了？是日期还是时间戳？为什么会有 camelCase、PascalCase 和 snake_case？带有 promise 的异步注解和又一个异步注解？如果失败，我们会返回 null？简直是魔鬼啊！

那时，每隔 5 分钟就有一半的公司同事向我发 Skype 消息，索取 ETA 修复。

所以首先，有人需要知道这里究竟发生了什么。

我很快意识到，该为此负责的不是一个人，而是三个人。很久以前，其中两个人离开了公司，而第三个人今天早上还没来。这真是经典场景。

根据 Git 的记录，这三个人碰这个文件的时间各自差了很久。因此他们留下了不一致的代码、不同的样式、不一样的 ECMAScript 版本和不同的 promise 处理方式。

不管怎样，在这段代码中，一切都是模棱两可的，一切都是不一致的。这是一个绝佳的反面案例，你应该尽一切可能避免这种情况。不用说，代码审查并没有覆盖到这里。

现在好了，我们必须快速重写它。

我得更改那些变量和函数的名称。不能再有歧义了。各处的 Async/Await 都要做成相同的方式。

我还要确保自己不会漏掉任何错误，结果再返回一个 null。如果出现了什么问题，这些错误肯定要破坏函数。异常应由上面的层来处理。

// hotfix// @todo rewrite the ticket module entirelyasync function getTicket(ticketUuid, ticketExpirationTimestamp) {  await validTicketExpiration(ticketUuid, ticketExpirationTimestamp)  const ticket = await getTicketByUuid(ticketUuid)  return ticket}

最好的解决方案是重写这个模块的一部分。这里的票证验证逻辑很糟糕。但这并不是最要紧的事情。

当务之急是找出并修复错误。

在更新代码后，真正的错误开始浮出水面。前一天所做的一个配置更改改变了票证创建行为。返回到先前的配置可以立即解决这个问题。

接下来的一周时间里，有问题的模块被完全重写。

3肉酱意面

很久很久以前，在一个遥远的星系中，我正在做一个代码干净整齐的产品。

作为优质产品，一切都在内部做好了优化。功能是用尽可能少的代码开发的。代码高度重视可读性。由注重整洁代码的工程师管理的代码审查流程确保了产品严格遵循所有最佳实践。

SOLID、DRY、KISS、YAGNI 和你可以想到的其他首字母缩写词，这里都能见得到。

即使做到这个地步，这个产品的某个特殊部分也会间歇性地崩溃。在一个冲刺期间，我终于设法安排出了时间来调查这件事。

很快，我意识到问题不在于产品。那些错误只有一个共同点：一个依赖项。那是一个通过内部工件处理的内部依赖项。

它由另一个团队管理，而且——令人惊讶的是——这段代码不是免费提供的。你必须先获得许可才能看到它。因此，我请求了访问权限来了解到底发生了什么事情。

然后我收到了一条 slack 消息，问我为什么要访问源码。

“你好！为什么你需要访问这个存储库？”

“你是什么意思？你知道我在这里工作吗？等等，我在路上。”

我突然出现在他面前后，终于拿到了访问该项目的权限。

我在其中看到一个文件，大小为 300KB。300KB 的文本，竟然有那么大。它已经有好几年没人碰过了。上次碰过它的那个人，我完全不认识。

简直是最可怕的魔鬼。

那是我一生中见过的规模最大的意大利面条代码。篇幅所限，我并没有把所有代码都放在这里。下面的代码只是我看到的那一坨东西的冰山一角。

小心阅读，它读起来扎眼。

// Thousands of lines of spaghetti codesif (global.Builder){    module.exports.buildPgs = function(pgs, options, limitNodes = 0){        var config = options.config || {};        var builded = [];        pgs.each(function(pg){            var supported = pg.prop('tagName') == "INPUT"                            && pr.attr['name'] == "file"                            && options.pgs.rel.active == ""                            && global.FileReader;            if (!supported || !pg.f || pg.f.length == 0)                return;            for (var i = 0; i < pg.f.length; i++)            {                builded.push({                    file: pg.f[i],                    instanceConfig: _.extend({}, config)                });                if (isFunction(options.before))                {                    var returned = options.before(pg.f.path);                    if (typeof returned === 'object' && global.status.in_progress)                    {                        if (returned.action == "skip")                        {                            var needsSkip = (typeof global.status.in_progress === 'boolean' && global.status.in_progress)                                            || _.hasAny("cancel", Global._quotes.BAD_DELIMITERS)                                            || str.indexOf(Global._delimiter) > -1;                            if(needsSkip) return;                        }                        else if (typeof returned.config === 'object')                        {                            var LOCAL_BUILDER = new global.Builder("/builder/" + options.module + "/" + options.module + );                            for(var s=p,a=p.matchIndex(o),shift=0,i=0;i<a.length;i++){                                var deepcopyfile = JSON.parse(JSON.stringify(pg.f[i].getRawValue()));                                LOCAL_BUILDER.build(deepcopyfile)                                LOCAL_BUILDER.onmessage = global.Notification("buildPg", deepcopyfile);                            }                        }                    }                }            }        });    }}// Thousands of lines of spaghetti codes

我甚至都没有尝试去碰这个恶魔之子。

在这类情况下，解决方案不是从代码中找出来的。我召开了一次小组会议，向他们解释具体情况。我的计划也很简单。

我们用一个已经可用的开源模块替掉了这个撒旦般的依赖项。与往常一样，这是一个大问题。必须做一些准备工作才能正确插入新的依赖项。

一开始的快速调查已经演变成持续几天的一项艰巨任务。

在会议桌那头，Scrum 主管很生气。讨论得越多，我越觉得想要不碰到该死的东西会非常困难。当我展示我们的处境后，讨论结束了，答案是不行。

“你只需要稍微动一动这个模块，把它修好就行了，然后我们会继续原本的工作。”

于是乎，我做了开发人员为代码质量和项目的可持续性应该做的额外工作。我说不行。我甚至走得更远。这是我职业生涯中的第一次，也是唯一一次，如果我被迫要辞职，我已经做好辞职的准备。

他们显然问了其他开发人员。大家都拒绝了。

由于这个问题的严重性，我争取到替换这个模块所需的时间。我为开源依赖项开发了一个小型适配器。然后我摆脱了那个被诅咒的依赖项。

此后，那个产品一切顺利，运行正常。

开发人员经常会抱怨意大利面条代码，这是有充分理由的。这是你能见过的最糟糕的代码。但无需大量投资即可确保你避免这种情况。

一开始，本文想写的是一个最佳实践的列表。

“作为开发人员，为什么以及如何应用最佳实践。”

不过上面这个标题很容易像大剂量安眠药一般令人昏昏欲睡，此外我出于两个原因更改了计划。

首先，对于我，特别是对你来说，先谈论后果会有趣很多。对开发人员来说，这很重要，因为这就是魔鬼代码的起源。此外，如果你可以为我的遭遇会心一笑，那也很好。

其次，互联网上已经有很多关于这个主题的文章。它们都有一个共同点，就是它们的内容都是从两本书中摘出来的。这两本书培养了几代开发人员。——罗伯特·马丁的《代码整洁之道》、史蒂夫·麦康奈尔的《代码大全》

你是否真的要缩短代码审查时间，并且再也不想搞出什么魔鬼代码？直接看原始资料就行，花点时间好好看完这两本书。

我发现《代码大全》的方法更易读、更实用。但是，尽管《代码整洁之道》非常复杂，但它教给我的知识不亚于甚至超过了《代码大全》。前者里面使用的代码是 Java 和 C++，但是谁在乎具体的语言呢？你在这本书里学到的是规则和编程理念。

用代码审查来验证代码是好事情。但是，如果你不确定为什么它是好的代码，那么到头来还是会出来你经历过的魔鬼代码。

原文链接：

https://www.jesuisundev.com/en/the-worst-pieces-of-code?fileGuid=vwdYpVcQyqDCYhCr