5秒减5斤？ 简单的！ 直接砍断你的手臂。

我们大多数人都熟悉 SMART 目标的概念：具体、可衡量、可实现、现实和及时。 你可能会争辩说砍掉一个人的手臂并不现实，但根据典型的 SMART 定义，它肯定是。 这就是 SMART 工程目标的缺陷。 我认为可持续发展需要添加到组合中。 简单地实现你的结果与以可持续的方式实现它 - 或者当涉及到代码时，可维护的方式是有区别的。

我们最好的一些教训来自我们工程师的内部 Slack 咆哮； 这是我在深入研究旧代码后分享的一篇好文章。

这是旧版本的代码：

/* * To do its job, the PDFer only needs - * 1. resource_id/order_id * 2. picasso.use_preflight feature flag * However, UpdateOrder and Thumbnailer need more and therefore it gets passed * more than what it needs. We'll have to refactor the flow to get past this * undesirable situation. To minimize the spillover of larger than needed objects * within this code, we use this method to return only whats needed. * This should also help in refactoring later. */function getWhatsRequired(msg) {  const id = msg.resource.id;  const required = {    id,    resource_type: msg.resource_type,    receiptHandle: msg.receiptHandle,    rawUri: '/' + id.substring(id.indexOf('_') + 1) + '/' + id + '_raw.pdf',    rawX1a: '/' + id.substring(id.indexOf('_') + 1) + '/' + id + '_raw.pdfx',    rawNonX1a:      '/' + id.substring(id.indexOf('_') + 1) + '/' + id + '_raw_non_x1a.pdf',    proofUri: '/' + id.substring(id.indexOf('_') + 1) + '/' + id + '.pdf',    proofX1a: '/' + id.substring(id.indexOf('_') + 1) + '/' + id + '.pdfx',    proofNonX1a:      '/' + id.substring(id.indexOf('_') + 1) + '/' + id + '_non_x1a.pdf',    usePreflight:      msg.launch_darkly_flags['picasso.use_preflight'] === 1 ? true : false,    strictPreflight:      msg.launch_darkly_flags['picasso.strict_preflight'] === 1 ? true : false,  };  return required;}

虽然代码完成了它需要做的工作——从某种意义上说，它没有被主动破坏——它本可以在一些地方做一些润色，这会对它的可读性和可维护性产生很大的影响 .

第一个问题

这个名字并没有真正告诉我发生了什么。 `getsWhatsRequired` 是一个几乎适用于任何获取和/或格式化数据的函数的名称。它没有告诉我为什么这个函数独立存在，也没有告诉我它在做什么特别有用。

第二个问题

在 URI 中发生了一整套重复。这段代码在这个函数中重复了 6 次。重复 6 次使推理变得更加困难，因为您要么需要阅读每次重复并确定它们是否在做完全相同的事情，要么您最终跳过它并假设它每次都做完全相同的事情。在这种情况下，它每六次都在做同样的事情，但你必须付出大量的精神能量才能弄清楚这一点。

第三个问题

在重复的代码中，有一个正在丢失的意图和目标。我们不是为了它而剪断字符串并添加斜线和其他东西，我们这样做是有特定目的的！但为什么？从这段代码来看，一点都不清楚。

第四个问题

那些双等于？他们是承重的！在一些小的代码清理过程中，我的一位同事将它们转换为 ESLint 建议的 === 严格相等运算符，因为没有任何理由认为这需要进行松散的相等检查。

事实证明，通过的值不是 1，它们实际上是“1”。这是非常不同的，这导致了一个事件，因为我们有一段时间没有发送任何文件。

可以通过以下两种方法之一进行改进：

1. 如果传入的值始终是字符串，则使用字符串表示而不是 ===。
2. 如果该值以字符串或数字的形式出现，并且有充分的理由继续发生这种情况，则使用双等号并记录此特定行为，以便未来的您/您的同行知道您依赖于这种非常具体的行为。

第五个（也是迄今为止最重要的选择）

如果您要定义变量然后立即返回它，您可以立即返回该值，而无需先将其分配给变量。这是一个小细节，但它使功能更短。

这是新代码：

/* * Returns the expected file locations for each of the raws and proofs before and after preflighting. */function getPathsAndInstructions(msg) {  const id = msg.resource.id;  const orderIdWithoutPrefix = id.substring(id.indexOf('_') + 1);  const baseURL = '/' + orderIdWithoutPrefix + '/' + id;  return {    id,    resource_type: msg.resource_type,    receiptHandle: msg.receiptHandle,    rawUri: baseURL + '_raw.pdf',    rawX1a: baseURL + '_raw.pdfx',    rawNonX1a: baseURL + '_raw_non_x1a.pdf',    proofUri: baseURL + '.pdf',    proofX1a: baseURL + '.pdfx',    proofNonX1a: baseURL + '_non_x1a.pdf',    usePreflight: msg.launch_darkly_flags['picasso.use_preflight'] === '1',    strictPreflight: msg.launch_darkly_flags['picasso.strict_preflight'] === '1'  };}

我在上面所做的大部分更改都很好地反映了我刚才提到的机会，但有一个领域我想花更多时间解释一下：

这是我介绍的两行代码来代替重复六次的代码。

  const orderIdWithoutPrefix = id.substring(id.indexOf('_') + 1);  const baseURL = '/' + orderIdWithoutPrefix + '/' + id;

您可以很容易地争辩说这两行可能是这样的一行：

const baseURL = '/' + id.substring(id.indexOf('_') + 1) + '/' + id;

那会更节省空间，对吧？正确的！那将是100％正确的！这就是上下文发挥作用的时候。你自己看这段代码是不会知道的，但是所有的订单资产？它们被上传到订单资产服务，文件夹是不带前缀的订单 ID。

知道不带前缀的订单 ID 在此上下文之外具有特定含义，我认为将 id 前缀切片与路径组合分开会更好。

因此，在详细批评了这段代码之后，我想真正清楚一些事情：

合并的这段代码未经修饰不是工程师的错。代表 EM 和参谋工程师发言，我们有责任制定标准并支持我们的同事编写经过精心打磨和可维护的代码 - 而我们在这里没有这样做。学到了宝贵的一课。

此外，这不适用于您尚未准备好合并的代码。 我第一次开始解决问题时编写的代码通常与我最终得到的代码大不相同。 不要花时间修饰你会在 15 分钟内丢弃的代码。

针对我的吐槽，我收到了同事们的以下宝贵意见，我完全同意：

“当我试图找出编写一段代码的“最佳”方法时，我不会陷入分析瘫痪，而是通常会首先开始编写最糟糕、最暴力、想到的任何东西 -工作的东西。 然后我将编写我的测试（如果我还没有使用 TDD 编写它们，我当然建议这样做）。 现在我有了一个安全网，代码正在做它需要做的事情，现在我可以清理它并确信它仍然有效。

关注七爪网，获取更多APP/小程序/网站源码资源！