本文最初发布于 jesuisundev.com 网站，经原作者授权由 InfoQ 中文站翻译并分享。

在本文中，我将向你展示我见过的一些最糟糕的代码，它们被称为“魔鬼代码”，会带来很严重的后果。然而，我们发现通过一些好的实践，你可以很容易规避它们。

“魔鬼代码”

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

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

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

模棱两可和前后矛盾

很久以前，我在一个清晨醒来，被世界末日般的景象吓了一跳。生产环境出现一个很大的错误，所有系统票证莫名其妙地返回“null”。到处都乱成一团。所有人都像无头苍蝇一样到处乱跑。

我跑到公司，冲向工作站，第一步是看 Kibana。但是，没有日志，什么都没有。为了查明原因，我决定追溯票证的创建路径。因此，我必须深入研究系统从诞生到现在创建的那些内部库。经过一番调查，我找到了问题来源之一的一堆文件。

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

// use id and expire to get ticket
async 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 entirely
async function getTicket(ticketUuid, ticketExpirationTimestamp) {
  await validTicketExpiration(ticketUuid, ticketExpirationTimestamp)
  const ticket = await getTicketByUuid(ticketUuid)


  return ticket
}

复制代码

最好的解决方案是重写这个模块的一部分。这里的票证验证逻辑很糟糕。但这并不是最要紧的事情。当务之急是找出并修复错误。

在更新代码后，真正的错误开始浮出水面。前一天所做的一个配置更改改变了票证创建行为。返回到先前的配置则可以立即解决这个问题。接下来的一周时间里，有问题的模块被完全重写。

肉酱意面

很久以前，我正在做一个代码干净整齐的产品。作为优质产品，一切都在内部做好了优化。功能是用尽可能少的代码开发的。代码高度重视可读性。由注重整洁代码的工程师管理的代码审查流程确保产品严格遵循所有最佳实践。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

复制代码

我甚至都没有敢去碰它。

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

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

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

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

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

因此，我在代码质量和项目可持续性方面做了额外的工作。我说不行，甚至已经做好辞职的准备。他们显然问了其他开发人员。大家都拒绝了。

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

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

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

驱魔

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

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

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

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

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

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

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

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

原文链接：

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