4、常见缺陷模式：空指针异常、资源泄漏、并发问题、边界条件错误、魔法数字与硬编码

好，咱们直接进入正题。代码审查时，我总会反复检查几类问题。它们就像代码里的「老油条」，几乎每个项目都能碰到。今天我把它们拎出来，一个个说清楚。

4.1 空指针异常：最熟悉的陌生人

空指针异常，Java 程序员的老朋友了。说白了，就是你在一个 null 上调了方法。我见过最离谱的一次，线上服务挂了，查了半天，原因是某个接口返回了 null，调用方没检查，直接 .toString()。

避坑指南： 我曾经接手过一个遗留系统，里面到处都是 if (obj != null) 的嵌套。代码读起来像俄罗斯套娃。后来我强制团队用 Optional 和 Objects.requireNonNull()，清爽多了。

审查时，我重点关注三点：

  • 方法返回值：文档说可能返回 null，调用方处理了吗？
  • 链式调用：a.b().c() 这种写法，中间任何一环是 null 就崩了。
  • 集合操作：从 Map 里 get() 出来的东西，别想当然认为非空。
// 不推荐
String name = user.getProfile().getName(); // 可能 NPE

// 推荐
String name = Optional.ofNullable(user)
    .map(User::getProfile)
    .map(Profile::getName)
    .orElse("默认用户");

4.2 资源泄漏：你关了吗？

资源泄漏，说白了就是「借了东西不还」。数据库连接、文件流、网络 socket，用完了得还回去。你想想看，一个连接池就那么大，你借一个不还，别人就没得用了。

我个人习惯，审查代码时看到 InputStream、Connection 这些关键字，立刻警觉。Java 7 之后有 try-with-resources，为什么不用？

核心原则： 谁打开，谁关闭。如果做不到，文档里必须写清楚谁负责关闭。 
// 不推荐
InputStream in = new FileInputStream("test.txt");
// 处理逻辑...
// 忘了 close()

// 推荐
try (InputStream in = new FileInputStream("test.txt")) {
    // 处理逻辑...
} // 自动关闭

我在项目中遇到过一个问题：一个定时任务，每天凌晨跑，跑了三个月，突然挂了。查了半天，是每次任务都打开了一个数据库连接，但没关闭。三个月下来，连接池满了，新任务进不来。嗯，这种问题最难排查，因为日志里什么异常都没有，就是卡死了。

4.3 并发问题：你以为的顺序，不一定是顺序

并发问题，是代码审查里的「硬骨头」。为什么？因为它不是每次必现。你测试一百次可能都正常，上线第一天就出问题。

我审查并发代码时，会问自己几个问题：

  • 这个变量会被多个线程同时读写吗？
  • 有没有加锁？锁的粒度够细吗？
  • 有没有用 volatile 保证可见性？
  • 集合类是不是线程安全的？
避坑指南： 我曾经见过一个同事，用 HashMap 做缓存，多个线程同时 put，结果死循环了。CPU 飙到 100%，服务器直接挂掉。后来改成 ConcurrentHashMap，问题解决。 
// 不推荐
private int count = 0;
public void increment() {
    count++; // 非原子操作，多线程下结果不对
}

// 推荐
private AtomicInteger count = new AtomicInteger(0);
public void increment() {
    count.incrementAndGet();
}

说白了，并发编程的核心就两件事：原子性和可见性。你写代码时，脑子里得有一张「多线程执行时序图」，想想最坏情况下会发生什么。

4.4 边界条件错误：差之毫厘，谬以千里

边界条件，就是代码的「临界点」。比如数组的最后一个元素、循环的终止条件、数值的最大最小值。这些地方最容易出 bug。

我审查时，会特别关注：

  • 循环边界：是 < 还是 <=？差一个数，结果天差地别。
  • 集合索引：list.get(list.size()) 会抛 IndexOutOfBoundsException。
  • 数值溢出：Integer.MAX_VALUE + 1 变成负数，你信不信？
小技巧： 写代码时，先想清楚「空集合」「单个元素」「最大/最小值」这三种情况。把这三种情况测过了，边界问题基本能覆盖。 
// 不推荐
for (int i = 0; i <= list.size(); i++) { // 越界！
    System.out.println(list.get(i));
}

// 推荐
for (int i = 0; i < list.size(); i++) {
    System.out.println(list.get(i));
}

我记得有一次，一个排序算法在数据量超过 1000 万时突然变慢。查了半天，是用了 int 做索引，数据量太大导致溢出，索引变成负数，算法直接乱套了。嗯，这种问题，光靠单元测试很难发现。

4.5 魔法数字与硬编码：代码里的「幽灵」

魔法数字，就是代码里直接出现的数字常量，比如 if (status == 3)。你看到这个 3，知道它代表什么吗？三个月后你自己都忘了。

我个人习惯，所有有业务含义的数字、字符串，必须用常量或枚举代替。这不是矫情，这是为了「可读性」和「可维护性」。

核心原则： 代码应该自解释。如果一段代码需要注释才能看懂，那说明代码本身写得不够好。 
// 不推荐
if (user.getAge() > 65) {
    // 老年人优惠
}

// 推荐
private static final int ELDERLY_AGE_THRESHOLD = 65;

if (user.getAge() > ELDERLY_AGE_THRESHOLD) {
    // 老年人优惠
}

硬编码的问题更隐蔽。比如把数据库连接字符串、API 密钥直接写在代码里。你想想看，如果换了环境，是不是得改代码重新部署？

缺陷类型 典型表现 审查要点
魔法数字 if (x > 100) 是否有常量定义？业务含义是否清晰？
硬编码路径 "/home/user/data.txt" 是否可配置？是否跨平台兼容？
硬编码密钥 "password123" 是否从环境变量或配置中心读取？
避坑指南： 我曾经在一个项目里发现，某个核心算法的阈值写死了 0.75。后来业务调整，需要改成 0.8，结果找了半天才找到这个数字。更坑的是，另一个地方也用了 0.75，但含义完全不同。从那以后，我要求团队所有数字常量必须命名。

好了，这五种缺陷模式，是我在代码审查中遇到最多的。你审查代码时，可以拿这个清单对照着看。下一章，咱们聊聊如何建立团队的代码审查文化。

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