【译文】高风险重构

|

代码重构如果操作不当，可能会造成巨大损失。功能失调的系统改造或新功能加上错误的重写，毫无疑问会造成损害。至于危害程度如何，人们可以争论不休。

每一次重构都隐藏着风险

代码改进是有风险的，因为它们旨在改变一个工作系统。这是事实。不过，在许多情况下，开发人员可以通过不同的活动来降低风险，使其变得可以承受。

有些重构任务规模较大，会影响许多子系统。相比之下，另一些重构任务则局限于单个组件，但可能会对整个系统的其他部分造成不可预见的影响，导致重要的业务运行瘫痪。例如，核心产品的现有采购流程。第三类是改进，为新功能 “腾出空间”–例如，改变单一产品的采购流程以支持更多项目，然后再增加一个新流程。

这三种情况的共同点是它们都意味着高风险。(1) 如果做错了，这种改进会损害业务（收入损失、客户流失）、团队（信任、动力）或任何相关功能（开发受阻）。(2) 另一方面，执行改进需要额外的细心、努力和时间，因此成本很高。经验丰富、具备领域知识的开发人员也是此类任务的首选。

应对风险（核对表）：

✅ 确定制约因素。我应该做到什么程度。
✅ 将改进从功能中分离出来。不要同时应用它们。
✅ 编写大量测试。更高级别的（集成）测试，减少实施细节。它们应与更改同时运行。
✅ 进行可视化确认。打开浏览器。

❌ 不要跳过测试。不要偷懒。
❌ 不要过于依赖代码审查和 QA。人都会犯错。
❌ 不要将昂贵的清理工作与其他更改混在一起。但对于小的改进要这样做。

有时，需要进行哪些改进以及改进在代码库中的什么位置是显而易见的。从组件中移出一些代码或进行额外的清理，这样新功能就不会踩在碎玻璃上。对于开发人员来说，这可能是迟早要解决的问题。但是，具有挑战性的重构（尤其是）隐藏着陷阱–由于开发环境障碍、依赖关系、数据库/API、测试不稳定或缺乏时间，验证变更并不简单。在整个改进过程中，也会经常出现故障。如何及早发现故障？

简单地开始重写很有诱惑力，但是…

重构

✋ 先等等

  • 从开发和业务角度评估变更的前期风险（成本）有多高。

如果错误百出的代码被发布，会发生什么？业务目标破灭、用户流失、指责或失去信任都是需要考虑的严重问题。考虑到可能出现的负面结果，是很好的第一步。也许一开始就不要重构。

  • 考虑改进是一项独立的任务，还是另一项功能的一部分。

例如，一个新功能的实现揭示了系统重构的明显候选方案，功能本身将直接受益。在这种情况下，当机立断，立即行动是切实可行的。通常，”以后 “永远不会到来。

但是，将清理工作从主要功能工作中分离出来，以便对它们进行单独审核和质量保证，一般来说是个不错的方法。尤其是当改进工作预计会产生副作用或难以验证时。

  • 事先证明系统正常运行。

在开始任何工作之前，验证所有相关部分是否正常运行至关重要。仅仅通过所有应用程序测试是不够的。这些测试涵盖了特定的用户流，但可能会遗漏其他用户流，因此这里的任务是通过适当的测试来填补空白。

🏋️ 重构是一件大事

因此，确保系统在与新代码交换后以同样的方式运行是非常重要的。在这方面，在整个重构过程中立即发现故障是非常有帮助的。没有人希望在生产过程中发现问题。因此编写全面的测试。这应该是自始至终的核心活动。

以测试的形式使用面包屑：

  • 单元测试和高级集成测试。
  • 不涉及实施细节。
  • 尽可能多地测试，以建立信心（也就是测试一切）。

集成测试在这里非常实用，它可以捕捉到泄漏组件的副作用。

// React component test.

it("should show all addons", () => {
  // Mocking the server response.
  // Util that mocks to the lowest possible level—window.fetch()
  server.get(endpoints.loadAddonsData, { addons: ["addon1", "addon2"] });
  // A prop controlling the component's button.
  props.shouldShowMoreAddons = true;

  // Render.
  render(<Addons {...props} />);

  // Clicking the button to show all addons.
  fireEvent.click(screen.getByRole("button"));

  // Assert the addons list is shown.
  await waitFor(() => {
    expect(screen.queryByRole("list")).toBeInTheDocument();
  });
})

一旦一切正常✅，就交由质量保证部门进行下一级验证。我们是人。

重构与功能相结合

如果时间紧迫，请单独发布功能，稍后再处理重构。这需要 QA 重新测试部分功能，但总比一次发布太多东西要好。

如果重构隐藏了太多未知因素、成本或增加的风险，无法承受，则应评估并选择不与新代码一起重构。否则，应确保系统在 I/O 上正常运行。稍作改进即可。

{}示例（代码）

下面的示例展示了一个 Dashboard React 组件，该组件渲染了一些小部件和一个upsell 框（<UpsellBox1 />），其数据依赖于后台调用（loadUpsell1Data()）。

// The example is greatly simplified.
export function Dashboard({ data }) {
  // Other business logic goes here
  const [shouldShowUpsell1, setShouldShowUpsell1] = useState(false);
  const [upsell1Data, setUpsell1Data] = useState(null);

  // Other business logic goes here
  // and here
  // and here
  // ...

  useEffect(() => {
    // Complex logic to determine if UpsellBox1 should show.
    if (condition1 && condition2 && !condition3) {
      setShouldShowUpsell1(true);

      loadUpsell1Data().then((bannerData) => {
        setUpsell1Data(bannerData);
      });
    }
  }, [...]);

  return (
    <div>
      <Widget1 />
      <Widget2 />
      ...
      {shouldShowUpsell1 && <UpsellBox1 data={upsell1Data} />}
    </div>
  );
}

想象一下，新任务是呈现第二个upsell （<UpsellBox2 />），如果两个都存在，则将它们放在转盘中。

return (
  <div>
    <Widget1 />
    <Widget2 />
    ...
    <!-- Display both upsell boxes -->
    {shouldShowUpsell1 && shouldShowUpsell2 && (
      <UpsellSlider>
        <UpsellBox1 data={upsell1Data} />
        <UpsellBox2 data={upsell2Data} />
      </UpsellSlider>
    )}
    <!-- OR show only <UpsellBox1 /> -->
    {shouldShowUpsell1 && <UpsellBox1 data={upsell1Data} />}
    <!-- OR <UpsellBox2 /> -->
    {shouldShowUpsell2 && <UpsellBox2 data={upsell2Data} />}
  </div>
);

由于有两个upsell （将来可能会更多），<Dashboard /> 中的逻辑越来越多。为了不牺牲明确性，将upsell 代码移到自定义钩子 useUpsellData（而不是 <UpsellSlider />）中是合理的，但这很棘手，因为小部件和upsell 数据混合在一起。

下面是更改后组件的外观：

// The example is greatly simplified.
export function Dashboard({ data }) {
  // ✅ Tests prior refactoring
  // ❓ The logic sets "conditions" which is needed in useUpsellData
  // Other business logic goes here

  // ✅ New tests
  // ❓ To ensure existing code doesn't break
  // ❓ Proving the new code works
  // 🧨 High risk—can break both upsells causing a heavy load on support and missed sales.
  // The hook-specific tests will overlap with the
  // Dashboard integration ones. That's OK.
  const {
    upsell1: { shouldShowUpsell1, upsell1Data },
    upsell2: { shouldShowUpsell2, upsell2Data },
  } = useUpsellData(conditions);

  // ✅ Tests prior refactoring especially if the logic
  // below is twisted with the upsells
  // ❓ To ensure existing code doesn't break
  // 🧨 High risk—can break important features that are hard to track.

  // Other business logic goes here
  // and here
  // and here
  // ...

  // ✅ This useEffect is gone now, but its implementation
  // should be tested as part of useUpsellData() 👆
  // ❓ To ensure existing code doesn't break
  // 🧨 High risk—can break the first upsell leading to no purchases.
  // useEffect(() => {
  //   if (condition1 && condition2 && !condition3) {
  //     setShouldShowUpsell1(true);
  //
  //     loadUpsell1Data().then((bannerData) => {
  //       setUpsell1Data(bannerData);
  //     });
  //   }
  // }, [...]);

  return (
    <div>
      <!--
        ✅ Tests prior refactoring for the widgets
        ❓ To ensure existing code doesn't break
        🧨 High|Medium risk—can break something important.
      -->
      <Widget1 />
      <Widget2 />
      ...
      <!--
        ✅ New tests for the carousel with two slides
        ❓ To verify the change
        🧨 High|Medium risk—can break existing functionality.
      -->
      {shouldShowUpsell1 && shouldShowUpsell2 && (
        <UpsellSlider>
          <UpsellBox1 data={upsell1Data} />
          <UpsellBox2 data={upsell2Data} />
        </UpsellSlider>
      )}
      <!--
        ✅ Tests prior refactoring for this specific case
        ❓ To ensure existing code doesn't break
        🧨 High|Medium risk—can break existing functionality.
      -->
      {shouldShowUpsell1 && <UpsellBox1 data={upsell1Data} />}
      <!--
        ✅ New tests
        ❓ To verify the change
        🧨 Medium risk.
      -->
      {shouldShowUpsell2 && <UpsellBox2 data={upsell2Data} />}
    </div>
  );
}

这些示例的目的是说明，如果在繁忙的组件中结合新功能，重构会多么不稳定。这些挑战主要来自 <Dashboard /> 的业务逻辑的紧密耦合特性，该逻辑将许多部件和部分作为其直接子节点来处理。

👍 我是否应该重构

👍 如果事情变得过于复杂，就进行重构，但如果无法证明其有效，就停止重构。

👍 在重构新功能的同时，对你预计会发生变化的地方进行重构，但在出现模式之前，👎复制粘贴是可以的。

👍 积极寻找新的方法来确保重构的可预测性，但 👎 保守地假设 QA 会发现所有的错误。

👍 将业务逻辑移出繁忙的组件，但如果唯一的争论点是 “这段代码看起来不对”，则要勇敢地保留遗留代码。

本文文字及图片出自 The High-Risk Refactoring

你也许感兴趣的：

发表回复

您的电子邮箱地址不会被公开。 必填项已用 * 标注