良好的重构 vs 不良的重构

多年来，我雇用了很多开发人员。其中不少人都坚信我们的代码需要进行大量重构。但问题是：在几乎所有情况下，其他开发人员发现他们新重构的代码更难理解和维护。通常情况下，重构后的代码速度更慢，错误更多。

但别误会我，重构本身并不是坏事。它是保持代码库健康的重要部分。问题在于，糟糕的重构确实很糟糕。而在试图改善代码时反而让情况变得更糟，这个陷阱是很容易掉进去的。

那么，让我们来看看什么是好的重构和坏的重构，以及如何避免成为那个所有人都不愿看到靠近代码库的开发人员。

重构的好、坏与丑

抽象是好事，也可能是坏事。关键在于知道何时以及如何应用它们。让我们来看看一些常见的陷阱，以及如何避免它们。

1. 大幅改变编码风格

我见过的最常见的错误之一是，开发人员在重构时完全改变了编码风格。这通常发生在某人来自不同的背景，或者对某种编程范式有强烈意见时。

举个例子。假设我们有一段需要清理的代码：

重构前：

// 🫤 这段代码可以更简洁

function processUsers(users: User[]) {
  const result = [];
  for (let i = 0; i < users.length; i++) {
    if (users[i].age >= 18) {
      const formattedUser = {
        name: users[i].name.toUpperCase(),
        age: users[i].age,
        isAdult: true
      };
      result.push(formattedUser);
    }
  }
  return result;
}

不良重构：

import * as R from 'ramda';

// 🚩 采用了完全不同的风格和库
const processUsers = R.pipe(
  R.filter(R.propSatisfies(R.gte(R.__, 18), 'age')),
  R.map(R.applySpec({
    name: R.pipe(R.prop('name'), R.toUpper),
    age: R.prop('age'),
    isAdult: R.always(true)
  }))
);

虽然这个重构版本可能会吸引函数式编程的爱好者，但它引入了一个新的库（Ramda）和完全不同的编码风格。对于不熟悉这种方法的团队来说，维护起来可能会非常痛苦。

良好重构：

// ✅ 更简洁且更符合常规
function processUsers(users: User[]): FormattedUser[] {
  return users
    .filter(user => user.age >= 18)
    .map(user => ({
      name: user.name.toUpperCase(),
      age: user.age,
      isAdult: true
    }));
}

这个版本通过使用更符合JavaScript惯用法的方法（如filter和map）来改进了原始代码。它更简洁，更易读，但没有引入完全不同的范式或外部依赖。

2. 不必要的抽象

有一次，我雇用的某人没有理解底层代码就添加了大量新的抽象。他们开始将不应被分组的内容分组，且这些内容正在（有意地）逐渐分化。他们还合并了一些不应合并的配置（不同的API需要不同的配置）。

重构前：

// 🫤 这段代码可以更简洁
function processUsers(users: User[]) {
  const result = [];
  for (let i = 0; i < users.length; i++) {
    if (users[i].age >= 18) {
      const formattedUser = {
        name: users[i].name.toUpperCase(),
        age: users[i].age,
        isAdult: true
      };
      result.push(formattedUser);
    }
  }
  return result;
}

不良重构：

// 🚩 这里的层次和抽象比必要的要多得多
class UserProcessor {
  private users: User[];

  constructor(users: User[]) {
    this.users = users;
  }

  public process(): FormattedUser[] {
    return this.filterAdults().formatUsers();
  }

  private filterAdults(): UserProcessor {
    this.users = this.users.filter(user => user.age >= 18);
    return this;
  }

  private formatUsers(): FormattedUser[] {
    return this.users.map(user => ({
      name: this.formatName(user.name),
      age: user.age,
      isAdult: true
    }));
  }

  private formatName(name: string): string {
    return name.toUpperCase();
  }
}

const processUsers = (users: User[]): FormattedUser[] => {
  return new UserProcessor(users).process();
};

这个重构引入了一个包含多个方法的类，虽然看起来更“面向对象”，但实际上更复杂，也更难一目了然地理解。

良好重构：

// ✅ 更简洁且更符合常规
const isAdult = (user: User): boolean => user.age >= 18;

const formatUser = (user: User): FormattedUser => ({
  name: user.name.toUpperCase(),
  age: user.age,
  isAdult: true
});

function processUsers(users: User[]): FormattedUser[] {
  return users.filter(isAdult).map(formatUser);
}

这个版本将逻辑分解为小的、可重用的函数，而不引入不必要的复杂性。

3. 引入不一致性

我见过一些开发人员试图通过更新代码库的一部分，让它与其他部分完全不同，以使其“更好”。这通常会导致其他开发人员在不同风格之间切换时感到困惑和沮丧。

假设我们有一个 React 应用程序，我们始终使用 React Query 来进行数据获取：

// 在整个应用程序中
import { useQuery } from 'react-query';

function UserProfile({ userId }) {
  const { data: user, isLoading } = useQuery(['user', userId], fetchUser);

  if (isLoading) return <div>Loading...</div>;
  return <div>{user.name}</div>;
}

现在，假设有个开发人员决定仅在一个组件中使用 Redux Toolkit：

// 一个单独的组件
import { useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { fetchPosts } from './postsSlice';

function PostList() {
  const dispatch = useDispatch();
  const { posts, status } = useSelector((state) => state.posts);

  useEffect(() => {
    dispatch(fetchPosts());
  }, [dispatch]);

  if (status === 'loading') return <div>Loading...</div>;
  return <div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>;
}

这种不一致性令人沮丧，因为它仅针对一个组件引入了一种完全不同的状态管理模式。

更好的做法是继续使用 React Query：

// 保持一致的方法
import { useQuery } from 'react-query';

function PostList() {
  const { data: posts, isLoading } = useQuery('posts', fetchPosts);

  if (isLoading) return <div>Loading...</div>;
  return <div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>;
}

这个版本保持了一致性，在整个应用程序中使用 React Query 进行数据获取。它更简单，不需要其他开发人员仅为一个组件学习新的模式。

记住，代码库中的一致性非常重要。如果需要引入新模式，首先考虑如何获得团队的支持，而不是制造单一的不一致性。

---

4. 在重构前不理解代码

我见过的最大问题之一就是在学习代码的过程中进行重构，以此来学习它。这是一个非常糟糕的主意。我见过一些评论建议你应该使用一段代码 6-9 个月。否则，你可能会制造错误，降低性能等。

重构前：

// 🫤 这里有点太多硬编码的东西了
function fetchUserData(userId: string) {
  const cachedData = localStorage.getItem(`user_${userId}`);
  if (cachedData) {
    return JSON.parse(cachedData);
  }

  return api.fetchUser(userId).then(userData => {
    localStorage.setItem(`user_${userId}`, JSON.stringify(userData));
    return userData;
  });
}

不良重构：

// 🚩 缓存机制去哪了？
function fetchUserData(userId: string) {
  return api.fetchUser(userId);
}

重构者可能认为他们简化了代码，但实际上他们移除了一个重要的缓存机制，这个机制是用来减少 API 调用和提高性能的。

良好重构：

// ✅ 保持现有行为的同时优化代码
async function fetchUserData(userId: string) {
  const cachedData = await cacheManager.get(`user_${userId}`);
  if (cachedData) {
    return cachedData;
  }

  const userData = await api.fetchUser(userId);
  await cacheManager.set(`user_${userId}`, userData, { expiresIn: '1h' });
  return userData;
}

这个重构保留了缓存行为，同时可能通过使用更复杂的缓存管理器（带有过期机制）进行了改进。

---

5. 理解业务背景

我曾经加入过一家公司，带着沉重的遗留代码负担。我领导了一个项目，将一家电子商务公司迁移到新的、现代的、更快更好的技术上……Angular.js。

结果发现，这个业务严重依赖于 SEO，我们构建了一个缓慢且臃肿的单页应用程序。

两年内我们除了一个速度更慢、错误更多、维护性更差的旧版网站的复制品外，什么也没发布。为什么？领导这个项目的人（是我——我是这个情境中的混蛋）之前没有在这个网站上工作过。我当时年轻且愚蠢。

让我们看一个现代例子来说明这个错误：

不良重构：

// 🚩 对于一个以 SEO 为重点的网站来说，单页应用是个坏主意
function App() {
  return (
    <Router>
      <Switch>
        <Route path="/product/:id" component={ProductDetails} />
      </Switch>
    </Router>
  );
}

这种方法看起来现代且整洁，但它完全是客户端渲染的。对于一个高度依赖 SEO 的电子商务网站来说，这可能是灾难性的。

良好重构：

// ✅ 为一个以 SEO 为重点的网站进行服务器渲染
export const getStaticProps: GetStaticProps = async () => {
  const products = await getProducts();
  return { props: { products } };
};

export default function ProductList({ products }) {
  return (
    <div>
      ...
    </div>
  );
}

这个基于 Next.js 的方法提供了开箱即用的服务器端渲染，这对 SEO 至关重要。它还提供了更好的用户体验，具有更快的初始页面加载速度，并提高了慢速连接用户的性能。Remix 也同样适合这种目的，提供了类似的服务器端渲染和 SEO 优化的好处。

6. 过度合并代码

我曾经雇佣了一个人，在他入职的第一天就开始立即重构代码。我们有一堆 Firebase 函数，其中一些的设置与其他的不同，比如超时时间和内存分配。

这是我们原始设置的样子。

重构前：

// 😕 我们在代码库中有 40 多次相同的代码，也许我们可以合并一下
export const quickFunction = functions
  .runWith({ timeoutSeconds: 60, memory: '256MB' })
  .https.onRequest(...);

export const longRunningFunction = functions
  .runWith({ timeoutSeconds: 540, memory: '1GB' })
  .https.onRequest(...);

这个人决定将所有这些函数封装在一个 createApi 函数中。

不良重构：

// 🚩 盲目合并不应当合并的设置
const createApi = (handler: RequestHandler) => {
  return functions
    .runWith({ timeoutSeconds: 300, memory: '512MB' })
    .https.onRequest((req, res) => handler(req, res));
};

export const quickFunction = createApi(handleQuickRequest);
export const longRunningFunction = createApi(handleLongRunningRequest);

这个重构将所有 API 设置为相同的设置，且无法为每个 API 单独覆盖。这是个问题，因为有时我们需要为不同的函数设置不同的配置。

更好的方法是允许每个 API 通过 Firebase 选项传递。

良好重构：

// ✅ 设定好的默认值，但允许任何人覆盖
const createApi = (handler: RequestHandler, options: ApiOptions = {}) => {
  return functions
    .runWith({ timeoutSeconds: 300, memory: '512MB', ...options })
    .https.onRequest((req, res) => handler(req, res));
};

export const quickFunction = createApi(handleQuickRequest, { timeoutSeconds: 60, memory: '256MB' });
export const longRunningFunction = createApi(handleLongRunningRequest, { timeoutSeconds: 540, memory: '1GB' });

通过这种方式，我们保持了抽象的好处，同时保留了我们所需的灵活性。在合并或抽象时，始终考虑你所服务的用例。不要为了“更干净”的代码而牺牲灵活性。确保你的抽象允许原始实现所提供的全部功能。

真的，在你开始“改进”代码之前要理解它。我们在下一次部署一些 API 时遇到了一些问题，这些问题本可以避免，如果没有这种盲目的重构。

---

如何正确重构

值得注意的是，你确实需要重构代码。但要正确地进行。我们的代码不是完美的，我们的代码需要清理，但要与代码库保持一致，熟悉代码，慎重选择抽象。

以下是一些成功重构的提示：

• 逐步进行：进行小的、可管理的更改，而不是大范围的重写。
• 深入理解代码，然后再进行重大重构或新的抽象。
• 匹配现有的编码风格：一致性是可维护性的关键。

• 避免过多的新抽象：除非确实需要复杂性，否则保持简单。
• 避免引入新的库，尤其是风格非常不同的库，除非团队同意。
• 在重构前编写测试，并随着重构的进行更新测试。这可以确保你保持了原有的功能。
• 让你的同事们对这些原则负责。
  

---

如何使用更好的工具和技术进行重构

为了确保你的重构是有益的而不是有害的，请考虑以下技术和工具：

Linting 工具

使用 linting 工具来强制一致的代码风格并捕捉潜在的问题。Prettier 可以帮助自动格式化为一致的风格，而 Eslint 则可以通过自定义插件帮助进行更细致的检查。

代码审查

实施彻底的代码审查，在合并重构代码之前从你的同事那里获得反馈。这有助于及早发现潜在问题，并确保重构后的代码符合团队标准和期望。

测试

编写并运行测试以确保重构后的代码不会破坏现有功能。Vitest 是一个特别快速、稳定且易于使用的测试运行器，默认情况下无需配置。对于视觉测试，可以考虑使用 Storybook。React Testing Library 是一组用于测试 React 组件的实用程序（还有 Angular 等变体）。

（正确的）AI 工具

让 AI 帮助你进行重构，至少那些能够匹配你现有编码风格和约定的工具。

一个特别有用的工具是 Visual Copilot。这款 AI 驱动的工具可以帮助你将设计转换为代码，同时匹配你现有的编码风格，并正确利用你的设计系统组件和令牌。

---

结论

重构是软件开发中必不可少的一部分，但需要经过深思熟虑，并尊重现有的代码库和团队动态。重构的目标是改善代码的内部结构，而不改变其外部行为。

记住，最好的重构通常对最终用户来说是不可见的，但会大大简化开发人员的工作。它们提高了可读性、可维护性和效率，而不会扰乱整个系统。

下次你感到有必要对一段代码进行“大计划”时，请停下来。彻底理解它，考虑你的更改的影响，并进行团队会感谢的小改进。

你未来的自己（以及你的同事）会感激你对保持代码库整洁和可维护性的深思熟虑的做法。