Java代码审查：在他人代码中寻找什么

为什么代码审查不是吹毛求疵，而是关乎质量

想象一下：你打开同事的拉取请求。里面有500行代码，散发着复制粘贴的味道，而且某个地方有人忘记关闭资源。你的任务不仅仅是找出错误，而是让代码变得更好。Java代码审查不是警察监管，而是共同创作。好的审查者能拯救团队免于技术债务，坏的审查者则会打击士气或遗漏漏洞。

在本文中，我将告诉你在Java代码审查中检查什么，以便不在琐事上浪费时间，但又不遗漏关键点。你将获得一份现成的检查清单和实践示例。

架构与设计：看本质

在深入细节之前，先评估整体情况。糟糕的架构无法通过格式化来修复。

违反SOLID原则

检查类是否变成了“上帝对象”。违反单一职责的示例：

public class OrderService {\n    public void createOrder(Order order) {\n        // 验证\n        // 计算折扣\n        // 发送邮件\n        // 日志记录\n        // 保存到数据库\n    }\n}\n

这里有五个修改原因。正确的做法是将日志记录和发送功能提取到单独的服务中。

过度复杂

有时代码用复杂的方式解决简单问题。问问自己：“能不能更简单？”如果可以——就在审查中写出来。记住KISS原则。

安全性：不可忽视的漏洞

Java代码评审必须包括安全检查。即使项目是内部项目。

SQL注入

字符串拼接是邪恶的：

// 不好的做法\nString query = "SELECT * FROM users WHERE name = '" + userName + "'";\nStatement stmt = connection.createStatement();\nResultSet rs = stmt.executeQuery(query);\n

正确的做法是使用PreparedStatement：

// 好的做法\nString query = "SELECT * FROM users WHERE name = ?";\nPreparedStatement pstmt = connection.prepareStatement(query);\npstmt.setString(1, userName);\nResultSet rs = pstmt.executeQuery();\n

敏感数据泄露

检查是否记录了密码、令牌或卡号。在Lombok中使用@ToString.Exclude或显式忽略。

不安全的对象比较

// 不好的做法（比较引用）\nif (password == storedPassword) { ... }\n// 好的做法\nif (password.equals(storedPassword)) { ... }\n

性能：瓶颈点

不要过早优化，但也不要忽略明显的性能问题。

循环中的不必要操作

// 不好的做法：每次调用length()\nfor (int i = 0; i < list.size(); i++) { ... }\n// 好的做法\nint size = list.size();\nfor (int i = 0; i < size; i++) { ... }\n

对简单任务使用Stream

有时普通循环更易读且运行更快：

// 过度使用\nlist.stream().filter(x -> x > 5).collect(Collectors.toList());\n// 简单做法\nList<Integer> result = new ArrayList<>();\nfor (int x : list) {\n    if (x > 5) result.add(x);\n}\n

内存泄漏

检查是否忘记关闭InputStream、Connection或Session。使用try-with-resources：

try (Connection conn = getConnection();\n     PreparedStatement stmt = conn.prepareStatement(sql)) {\n    // 工作\n} catch (SQLException e) {\n    log.error("DB error", e);\n}\n

可读性与风格：代码如书

代码审查检查清单必须包括可读性。代码写一次，阅读数十次。

魔法数字和字符串

// 不好的做法\nif (status == 3) { ... }\n// 好的做法\nprivate static final int STATUS_COMPLETED = 3;\nif (status == STATUS_COMPLETED) { ... }\n

长方法