代码坏味道（上）：重复代码、过长函数、过大的类、过长参数列表

好，咱们今天聊聊代码里的「坏味道」。

这个词不是我发明的，Martin Fowler 在《重构》里提的。但我做了这么多年 Node.js 项目，发现这些坏味道在 JavaScript 世界里尤其常见。为什么呢？因为 JS 太灵活了，灵活到你可以写出各种「能跑但很糟糕」的代码。

我个人习惯是，每次接手一个老项目，先不急着看业务逻辑。先扫一遍代码，闻一闻。嗯，味道不对的地方，往往就是问题最多的地方。

一、重复代码：最直观的坏味道

重复代码，说白了就是「复制粘贴」的产物。你想想看，一段逻辑在三个地方出现，改一个地方忘了改另外两个——这种事我遇到过太多次了。

核心判断标准： 如果一段代码需要同时修改多处，那就是重复代码。

来看个例子。这是我在一个电商项目里看到的真实代码：

// 坏味道：重复代码
function validateUserName(name) {
  if (!name || name.length < 2) {
    throw new Error('用户名至少2个字符');
  }
  if (name.length > 20) {
    throw new Error('用户名不能超过20个字符');
  }
  if (!/^[a-zA-Z0-9_]+$/.test(name)) {
    throw new Error('用户名只能包含字母、数字和下划线');
  }
}

function validateProductName(name) {
  if (!name || name.length < 2) {
    throw new Error('产品名至少2个字符');
  }
  if (name.length > 50) {
    throw new Error('产品名不能超过50个字符');
  }
  if (!/^[a-zA-Z0-9_\u4e00-\u9fa5]+$/.test(name)) {
    throw new Error('产品名只能包含字母、数字、下划线和中文');
  }
}

看出来了吗？两个函数的结构一模一样，只是错误信息和长度限制不同。这就是典型的重复代码。

怎么改？抽一个通用函数出来：

// 重构后：消除重复
function validateName(name, options) {
  const { minLen = 2, maxLen = 20, pattern, fieldName = '字段' } = options;
  
  if (!name || name.length < minLen) {
    throw new Error(`${fieldName}至少${minLen}个字符`);
  }
  if (name.length > maxLen) {
    throw new Error(`${fieldName}不能超过${maxLen}个字符`);
  }
  if (pattern && !pattern.test(name)) {
    throw new Error(`${fieldName}格式不正确`);
  }
}

// 使用
validateName(userName, { maxLen: 20, pattern: /^[a-zA-Z0-9_]+$/, fieldName: '用户名' });
validateName(productName, { maxLen: 50, pattern: /^[a-zA-Z0-9_\u4e00-\u9fa5]+$/, fieldName: '产品名' });
我的小技巧： 写代码时如果发现自己在「复制-粘贴-改参数」，立刻停下来。这通常意味着你需要一个抽象。

二、过长函数：一个函数超过20行就该警惕了

我见过最夸张的一个函数，800多行。一个函数，处理了用户登录、商品查询、订单支付……你敢信？

为什么会这样？因为很多人觉得「写短函数麻烦」，或者「拆成多个函数调用起来费劲」。但你想过没有，一个800行的函数，你改一行代码要读完全部800行才能确定不会出问题。

我曾经…… 在一个遗留系统里改一个bug，那个函数400多行。我花了整整两天才理清楚逻辑，最后发现只是少了一个空值判断。从那以后，我给自己定了个规矩：一个函数超过30行，必须拆。

过长函数的典型特征：

  • 一个函数里既有数据校验，又有业务逻辑，还有数据库操作
  • 函数内部有大量注释来解释「这一段在做什么」
  • 缩进层次超过3层（if 套 if 套 if）

看这个例子：

// 坏味道：过长函数
async function createOrder(userId, items, address) {
  // 校验用户
  const user = await User.findById(userId);
  if (!user) throw new Error('用户不存在');
  if (user.status !== 'active') throw new Error('用户已禁用');
  
  // 校验商品
  for (const item of items) {
    const product = await Product.findById(item.productId);
    if (!product) throw new Error(`商品${item.productId}不存在`);
    if (product.stock < item.quantity) throw new Error(`商品${product.name}库存不足`);
    // 还有价格计算、优惠券检查、运费计算...
  }
  
  // 创建订单
  const order = new Order({ userId, items, address, status: 'pending' });
  await order.save();
  
  // 扣减库存
  for (const item of items) {
    await Product.updateOne({ _id: item.productId }, { $inc: { stock: -item.quantity } });
  }
  
  // 发送通知
  await sendEmail(user.email, '订单创建成功');
  await sendSMS(user.phone, '您的订单已提交');
  
  return order;
}

这个函数干了四件事：校验、创建订单、扣库存、发通知。每一件事都应该是一个独立的函数。

// 重构后
async function createOrder(userId, items, address) {
  await validateUser(userId);
  await validateItems(items);
  
  const order = await buildOrder(userId, items, address);
  await deductStock(items);
  await notifyUser(userId, '您的订单已提交');
  
  return order;
}

async function validateUser(userId) { /* ... */ }
async function validateItems(items) { /* ... */ }
async function buildOrder(userId, items, address) { /* ... */ }
async function deductStock(items) { /* ... */ }
async function notifyUser(userId, message) { /* ... */ }
记住： 函数名就是最好的注释。如果需要一个注释来解释「这一段在做什么」，那这段代码就应该是一个独立的函数。

三、过大的类：一个类超过200行就该反思了

过大的类和过长函数是「近亲」。一个类里塞了太多不相关的职责，就成了「上帝类」——什么都能干，但什么都干不好。

判断标准很简单：

  • 类的名字里出现「And」或者「Util」——比如 UserAndOrderManager、CommonUtils
  • 类的方法超过15个
  • 类的字段超过10个
  • 你很难用一句话说清楚这个类是干什么的

举个例子，我见过一个 UserService 类，里面包含了：

  • 用户注册、登录、找回密码
  • 发送邮件、短信通知
  • 生成验证码、校验验证码
  • 记录操作日志
  • 甚至还有图片上传功能

这明显违反了「单一职责原则」。一个类应该只有一个引起它变化的原因。用户登录逻辑变了要改这个类，图片上传逻辑变了也要改这个类——这不合理。

我的做法： 每当我发现一个类里出现了「而且」这个词，比如「这个类处理用户登录而且处理图片上传」，我就知道该拆了。

四、过长参数列表：超过3个参数就该考虑封装了

函数参数太多，调用的时候你自己都记不清顺序。我见过一个函数，7个参数，每次调用都要翻回去看参数定义。

过长参数列表的典型场景：

  • 配置类参数：比如 timeout、retry、cache 这些
  • 可选参数：有些参数大部分时候用默认值，但偶尔需要改
  • 数据聚合：多个参数其实属于同一个业务概念
// 坏味道：过长参数列表
function createUser(name, email, phone, age, gender, address, avatar, role, status) {
  // 9个参数，调用时痛苦不堪
}

// 调用
createUser('张三', 'zhangsan@example.com', '13800138000', 25, 'male', 
           '北京市朝阳区', 'avatar.jpg', 'user', 'active');

怎么改？把相关的参数封装成对象：

// 重构后：参数对象化
function createUser(userInfo) {
  const { name, email, phone, age, gender, address, avatar, role = 'user', status = 'active' } = userInfo;
  // ...
}

// 调用
createUser({
  name: '张三',
  email: 'zhangsan@example.com',
  phone: '13800138000',
  age: 25,
  gender: 'male',
  address: '北京市朝阳区',
  avatar: 'avatar.jpg'
});

你看，这样调用的时候，每个参数都有名字，顺序也不重要了。而且默认值可以在解构时直接设置，调用方可以省略不传。

一个实用原则： 如果函数参数超过3个，或者有超过2个可选参数，就用对象参数。这是 Node.js 社区的主流做法，也是我个人强烈推荐的。

小结一下

这四种坏味道，说白了都是「懒」的产物。懒得抽函数、懒得拆类、懒得封装参数。但你要知道，代码写出来是给人看的，其次才是给机器执行的。

我每次 code review 的时候，看到重复代码、过长函数、过大的类、过长参数列表，基本就能判断出这个模块的质量了。嗯，八九不离十。

下一节咱们继续聊剩下的几种坏味道——switch 语句、临时字段、过度耦合这些。到时候见。

← 上一章 📖 返回目录 下一章 →