我见过最糟糕的代码

数学算法俱乐部

共 5625字，需浏览 12分钟

· 2021-04-24

数学算法俱乐部

日期： 2021年04月21日

正文共：3192字

来源： infoQ

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

“魔鬼代码”

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

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

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

模棱两可和前后矛盾

很久以前，我在一个清晨醒来，被世界末日般的景象吓了一跳。生产环境出现一个很大的错误，所有系统票证莫名其妙地返回“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}

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

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

肉酱意面

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