Java 代码审查清单——我们团队强制要求检查的 15 个代码质量项
Java 代码审查清单——我们团队强制要求检查的 15 个代码质量项
适读人群：有 code review 习惯或想建立 code review 规范的 Java 团队 | 阅读时长：约 16 分钟 | 核心价值：15 个真实在用的 code review 检查项，每项附带反例和正例
我们团队推行严格的 code review 已经三年了。
这三年，因为 code review 被提前发现的 bug 数量超过了上线后发现的 bug 数量。更重要的是，它让团队的代码风格越来越统一，新人上手也越来越快。
但 code review 有个问题：如果只是"我觉得你这里写得不好"，没有具体依据，很容易变成争论风格而不是讨论质量。有了清单，就有了共同语言。
这篇文章整理了我们团队当前在用的 15 个强制检查项。不是网上那种泛泛而谈的"要写注释"，每一条都附带我见过的真实反例。
检查项 1：空指针防御
检查点： 对从外部获取的数据（方法参数、数据库结果、接口返回）的 null 处理
反例：
// 没有 null 检查，一旦 getUser() 返回 null，NPE
String userName = userService.getUser(userId).getName();正例：
package com.example.review;
import java.util.Optional;
public class NullSafeExample {
// 方式一：Optional 链式处理
public String getUserNameSafe1(Long userId) {
return Optional.ofNullable(userService.getUser(userId))
.map(User::getName)
.orElse("未知用户");
}
// 方式二：显式 null 检查（逻辑更清晰）
public String getUserNameSafe2(Long userId) {
User user = userService.getUser(userId);
if (user == null) {
return "未知用户"; // 或者 throw new UserNotFoundException(userId)
}
return user.getName();
}
}检查项 2：资源关闭
检查点： 所有实现 Closeable 的资源（文件、流、连接）必须在 try-with-resources 里使用
反例：
// 一旦 process() 抛异常，conn 永远不会被关闭
Connection conn = dataSource.getConnection();
process(conn);
conn.close();正例：
try (Connection conn = dataSource.getConnection()) {
process(conn);
} // conn 在这里自动 close，无论是否异常检查项 3：异常处理不能吞掉
检查点： catch 块里不能只有注释或空语句，不能静默忽略异常
反例：
try {
doSomething();
} catch (Exception e) {
// TODO: 处理异常
// 或者更糟：什么都没有
}正例：
// 要么处理，要么重新抛出，要么记录日志（三选一，或组合）
try {
doSomething();
} catch (Exception e) {
log.error("执行 doSomething 失败，参数: {}", param, e); // 记录完整异常
throw new BusinessException("操作失败", e); // 或者重新抛出
}检查项 4：不要在循环里查数据库
检查点： for/while 循环体内不能有数据库查询、HTTP 调用、Redis 操作
反例：
// 1000 个 userId，查了 1000 次数据库——N+1 问题
for (Long userId : userIds) {
User user = userRepository.findById(userId); // 循环里查库！
result.add(buildVO(user));
}正例：
package com.example.review;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
// 一次查出所有，再在内存里处理
public class BatchQueryExample {
public List<UserVO> buildUserVOs(List<Long> userIds) {
// 一次查出所有需要的 User
List<User> users = userRepository.findAllById(userIds);
Map<Long, User> userMap = users.stream()
.collect(Collectors.toMap(User::getId, u -> u));
// 在内存里做映射，不再查库
return userIds.stream()
.map(id -> buildVO(userMap.get(id)))
.collect(Collectors.toList());
}
}检查项 5：equals 和 hashCode 必须同时重写
检查点： 如果重写了 equals()，必须同时重写 hashCode()
如果只重写 equals 不重写 hashCode：对象可能 equals() 为 true，但放进 HashMap 里找不到，因为 hashCode 不同导致哈希桶不同。
检查项 6：集合操作的线程安全
检查点： 在多线程环境中，使用 ConcurrentHashMap、CopyOnWriteArrayList 等线程安全集合，而不是 HashMap + synchronized
反例：
// HashMap 不是线程安全的，多线程并发 put 可能导致死循环（Java 7）或数据丢失（Java 8）
private static final Map<String, Object> CACHE = new HashMap<>();
// 或者用了 synchronized 但范围不对
public void put(String key, Object value) {
synchronized (this) { CACHE.put(key, value); } // 看起来安全，但 CACHE 是 static，this 锁不够
}正例：
private static final Map<String, Object> CACHE = new ConcurrentHashMap<>();检查项 7：不要用 Executors 工厂方法创建线程池
检查点： 禁止使用 Executors.newFixedThreadPool()、Executors.newCachedThreadPool() 等，要用 ThreadPoolExecutor 显式配置
反例：
ExecutorService executor = Executors.newFixedThreadPool(10);
// 问题：队列是无界的 LinkedBlockingQueue，任务堆积时可能导致 OOM正例：
package com.example.review;
import java.util.concurrent.*;
public class SafeThreadPoolExample {
// 显式配置所有参数，不用默认的危险值
private static final ThreadPoolExecutor EXECUTOR = new ThreadPoolExecutor(
4, // 核心线程数
8, // 最大线程数
60, // 空闲线程存活时间
TimeUnit.SECONDS,
new LinkedBlockingQueue<>(1000), // 有界队列，防止 OOM
new ThreadFactory() {
private int count = 0;
public Thread newThread(Runnable r) {
Thread t = new Thread(r, "biz-thread-" + count++);
t.setDaemon(false); // 非守护线程
return t;
}
},
new ThreadPoolExecutor.CallerRunsPolicy() // 队列满时，由调用线程执行
);
}检查项 8：魔法数字必须定义为常量
反例：
if (status == 2) { // 2 是什么含义？
sendEmail();
}正例：
private static final int STATUS_VERIFIED = 2; // 状态：已验证
if (status == STATUS_VERIFIED) {
sendEmail();
}检查项 9：日志记录要有上下文，不要只记录异常消息
反例：
} catch (Exception e) {
log.error(e.getMessage()); // 只有异常消息，没有上下文
}正例：
} catch (Exception e) {
log.error("处理订单失败，orderId={}, userId={}", orderId, userId, e); // 有完整异常栈和关键参数
}检查项 10：BigDecimal 比较必须用 compareTo
反例：
BigDecimal a = new BigDecimal("1.0");
BigDecimal b = new BigDecimal("1.00");
System.out.println(a.equals(b)); // false！精度不同，equals 返回 false正例：
System.out.println(a.compareTo(b) == 0); // true，compareTo 忽略精度差异检查项 11：API 接口的参数校验必须显式做
检查点： Controller 接收的请求参数，必须用 @Valid + Bean Validation 注解校验，不能假设参数合法
package com.example.review;
import jakarta.validation.Valid;
import jakarta.validation.constraints.*;
import org.springframework.web.bind.annotation.*;
@RestController
public class OrderController {
@PostMapping("/orders")
public OrderVO createOrder(@RequestBody @Valid CreateOrderRequest request) {
// @Valid 触发自动校验，不合法直接返回 400
return orderService.create(request);
}
}
class CreateOrderRequest {
@NotBlank(message = "商品 ID 不能为空")
private String productId;
@Min(value = 1, message = "数量至少为 1")
@Max(value = 100, message = "单次最多购买 100 件")
private Integer quantity;
@NotNull(message = "收货地址不能为空")
@Valid // 嵌套校验
private AddressRequest address;
}检查项 12：事务方法不能是 private 或 final
Spring 的 @Transactional 基于 AOP 代理，private 和 final 方法不能被代理，@Transactional 会静默失效。
// 错误：@Transactional 在 private 方法上不生效
@Transactional
private void doTransactional() { ... }
// 正确：public 方法，且不能是 final
@Transactional
public void doTransactional() { ... }检查项 13：同类内部调用绕过 @Transactional
反例：
@Service
public class UserService {
public void outerMethod() {
this.transactionalMethod(); // 直接调用，不走 AOP 代理，@Transactional 失效
}
@Transactional
public void transactionalMethod() { ... }
}正例： 通过 Spring 容器获取代理对象，或者把事务方法移到另一个 Bean 里。
检查项 14：Stream 里不要有副作用
检查点： Stream 的 map()、filter() 等操作里，不能有修改外部状态的代码
// 错误：在 forEach 里修改外部变量（并行 Stream 时有线程安全问题）
List<String> results = new ArrayList<>();
items.stream().map(this::process).forEach(results::add); // 如果是 parallelStream，有问题
// 正确
List<String> results = items.stream().map(this::process).collect(Collectors.toList());检查项 15：接口返回的数据结构要向前兼容
检查点： 接口升级时，不能删除已有字段，不能改变已有字段的类型，只能新增字段
这不是 Java 代码本身的问题，是 API 设计规范。我见过太多因为改了接口字段导致客户端崩溃的事故。
每次 code review 都要问一句：这个接口有没有已有的调用方？如果有，这次改动是否向前兼容？
这 15 条不是全部，但是我们实际执行中最有价值的一部分。建立清单的意义不是增加 review 负担，是让大家对"什么是好代码"有共同认知。