我见过的最糟糕代码

图0: 编程我见过的最糟糕代码

作者 | 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 codes
if (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