ARTICLE · INTELLIGENCE

战地情报 · 详情页

来自尧图项目组的一线实战观察与深度解析

代码评审的标准:T1/T2/T3三层质量模型,从能跑到能扛

代码评审的标准:T1/T2/T3三层质量模型,从能跑到能扛 这几年我参加过的代码评审没有一千也有八百场最怕听到的一句话不是这段代码有 bug而是这段代码写得不错。因为大部分人把不错等同于能跑、测试过了、能合入但在我自己十年代码生涯里能跑和一段代码真正意义上的好中间隔着的距离远比想象中大。我管自己这套判断标准叫 T3 Code。别误会这不是某个开源框架的名字也不是什么行业认证它只是我在评审、重构、带人的过程中沉淀出的一套代码质量分层模型。核心动作很简单把任意一段代码丢进这套模型先判断它处于 T1、T2、T3 哪个层级再决定能不能合入、要不要重构、问题优先级排多高。它解决的是所有研发团队都会遇到的困局——好代码这词太虚了我们需要一把能吵架、能达成共识的标尺。这篇文章不聊空理论重点讲四件事三个层级怎么定义、每层代码具体长什么样、一段支付回调代码的完整升级过程以及我在团队落地这套方法时踩过的坑。对刚入行的工程师它能帮你少走弯路对带团队做评审的它可以直接拿来做评审讨论的共同语言。1. T3 Code 的出身一场让我收回能跑二字的线上事故1.1 一次被评审全票通过的回调代码先说个真实的事。几年前我维护一个交易系统外部支付渠道在用户完成支付后会向我们的服务打一个异步回调。当时的实现长这样收到请求验签通过查订单如果订单状态是待支付就把订单改成已支付然后给用户账户加余额。评审时大家一致觉得这段代码写得到位——验签有了、状态判断有了、成功失败分支都有了返回格式也统一。我当时给的结论是能跑。上线第二周出事了。支付渠道因为机房网络抖动重发了回调而我们的代码在订单已经是已支付状态时直接返回成功、什么都不做。表面上看重复回调被无害化处理了对吧问题在于第一次回调执行的时候订单状态更新成功了但加余额那一步因为数据库连接池被占满而抛异常。重发的回调到了以后看到订单状态已经是已支付直接跳过加余额。于是用户钱付了余额没到账没有任何补偿渠道和日志还是用户打客服电话我们才知道出了问题。这段代码每个判断单独拆出来看都是对的。它的问题恰恰出在只对了快乐路径只要每一步都按剧本走结果就是对的一旦某一步断在半路整个流程没有任何兜底。这就是我后来定义的 T1 层代码——正确性依赖环境每一步都配合而不是靠设计保证。1.2 三个层级的定义与判断标准那场事故之后我开始把代码按可预期的程度分成三档。T1 是能跑输入符合预期时流程正确输入一偏、环境一抖行为就不可预期。典型特征包括深层嵌套、裸奔的类型、只处理成功分支、出错靠 console.log。T2 是能读结构清楚、命名准确、函数边界合理新人花两分钟能看懂改需求时有明确的落点。T3 是能扛非法状态在入口就被拦下失败路径都有显式出口关键行为有自动化测试兜底。层级一句话定位判断问题上线风险T1能跑换个输入还会对吗平时没事一出事就是大事T2能读新人两分钟能看懂吗可维护但正确性靠人肉T3能扛出错时行为可预期吗出问题也容易定位和回滚我在团队里反复强调一个态度T1 不是骂人T2 也不是万能。写个一次性脚本T1 完全够用但任何要进生产环境、跟钱和用户数据打交道的代码至少要 T2核心路径必须冲 T3。问题是大多数团队从不在评审里明确这段代码是 T1所以 T1 的代码才有机会在生产环境长期居住。2. T1 层一坨能跑的代码是怎么悄悄长大的2.1 T1 代码的几种典型长相我把这些年见过的高频 T1 形态整理了一下基本逃不出这五类。第一回调函数里直接堆四层 if。订单状态、支付结果、验签结果、是否重复全部揉在一个函数里缩进一层套一层没人敢拆因为动一处就可能牵走另一处。第二外部接口进来的数据一律不解析、不校验。路由处理函数里直接req.body.orderId字段名拼错了要等线上请求来了才知道。第三错误处理只有两种姿势catch 住以后 console.log 一下继续跑或者 catch 住以后什么都不做。第四魔法值随处可见。订单状态 2 代表什么错误码 10086 又代表什么全靠上下文猜。第五没有测试。单元测试不写集成测试嫌重回归全靠手点页面碰运气。这五类特征的共同点是一样的代码的正确依赖写代码的那个人当时脑子里的假设而这些假设没有任何东西能显式地传下来。2.2 T1 代码为什么越改越贵一地 T1 代码对组织的真正杀伤力不在于它现在有 bug而在于后续每次改动都在变贵。改一个字段你得先把整个函数从头到尾读一遍读完之后还不敢确认有没有漏掉的调用点因为类型信息已经丢了改完以后没有测试兜底只能提心吊胆点几下页面觉得看起来没问题。你可以把 T1 代码想象成一栋没有图纸的老房子灯能亮水管能出水居住没问题。但要加一盏灯就得凿开墙一寸一寸找电线找出电线又接到哪路开关上了只能试。每一次改造都在增加下一次改造的成本。团队里说的技术债说白了就是大量 T1 代码在核心路径上的叠加。2.3 也得说句公道话T1 不是原罪我不想把 T1 讲成十恶不赦。原型验证、一次性数据迁移、临时排障脚本这些场景写 T1 反而是最优解——它们不需要应付变化也不需要别人长期维护。T1 真正的问题不是存在而是没有被识别尤其当它悄悄躺在生产环境的钱相关路径上。所以我在团队里强调的第一步不是消灭 T1而是学会在评审时大声说出来这段代码目前是 T1。先有认识再谈改造。3. T2 层从写给自己看到写给团队看3.1 命名把注释写进名字里而不是写在名字旁边很多人以为 T2 的第一步是分清函数长短我的经验是先改命名。T1 代码里最普遍的信号就是变量名与语义脱节getData返回的到底是订单还是用户信息st 2是什么状态如果一段代码需要你点进函数实现才能确定变量含义它就是给一个人看的不是给团队看的。我习惯在评审时做命名对照实验把代码里的变量换成带业务语义的名字再看一次。d换成orderst 2换成status paid这不仅仅是可读性的提升也在逼你确认语义。别小看这一步很多逻辑 bug 就是在改名过程中暴露的当你必须说清楚这个字段的真实含义时那些顺手用一下的错误字段就藏不住了。3.2 函数边界跟着需求变更的落点走函数怎么拆算合理我听过很多答案单一职责、不要超过五十行、纯函数优先。这些原则对但不够实用。我自己最常用的判断标准是跟随需求变更的落点。问你一个简单的问题——最近一次需求变更你改了几处代码分属几个文件如果你的回答是改动集中在一个函数里十分钟搞定那这个模块的函数边界大概率是舒服的。如果你的回答是改一个字段要动五个文件还漏了一个那不管函数多短、职责多单一边界都是有问题的。拆函数不是做雕刻拆到每个函数都能独立讲一个业务故事就够了。过度拆分在团队里同样会变成负担每个小函数都是新的认知入口函数数量本身就是一种复杂度。3.3 判断 T2 最实用的一招模拟一次需求变更比任何原则都管用的评判方式是拿真实需求做一次思维实验。拿到一段代码先不看它写得好不好而是假设产品下周一要求加一个业务场景比如支付成功之后还要给用户发一个站内信。然后你自己在心里完成变更要改哪个函数、加哪个参数、在哪里调用、会不会影响别的分支。如果这个过程清晰流畅没有需要拆墙找线的恐惧它就是 T2 了。这个方法在评审会上效果特别好。很多开发嘴上说代码没毛病模拟一次变更就开始支支吾吾这个逻辑我好像得回去查一下这个字段我记得别的地方也用到过。不用我下结论他自己心里已经知道这段代码到不了 T2。4. T3 层让代码在坏运气面前仍然讲道理4.1 用类型系统先干掉一整类非法状态T3 层跟 T2 层最大的分水岭在哪里我的答案是正确性不再依赖人肉记忆。具体来说第一步是把该约束的用类型系统约束住。最有代表性的是联合类型和可辨识联合。比如支付回调的result字段通道方文档里写了success/failed两种取值T1 代码里直接当字符串处理T2 代码加个宽松的字符串类型而 T3 代码会在入口处就把它解析成联合类型type PaymentResult success | failed function parsePaymentResult(raw: unknown): PaymentResult | null { if (raw success || raw failed) return raw return null }别小看这一步。raw 经过解析之后后续所有分支都不需要再担心万一传进来一个 pending 会怎样因为那种取值在类型层面就被排除了。更好的是配合 switch 的穷尽检查——所有分支处理完加一个never位置兜底编译器会在有人新增类型时强迫你补齐逻辑function apply(paymentResult: PaymentResult) { switch (paymentResult) { case success: // 处理成功路径 break case failed: // 处理失败路径 break default: const _exhaustive: never paymentResult } }这一小段代码背后是我觉得 T3 层最核心的理念与其靠测试在运行期撞出问题不如让非法状态根本进不了系统。类型就是一份编译器替你执行的文档。4.2 错误处理把预期失败和非预期失败分开T1 代码的错误处理通常只有两个动作——吞掉或打日志。T3 层要求你把失败分成两类分别用不同的策略对待。第一类是预期失败请求体格式不对、验签失败、订单不存在、重复回调。这些是业务流程的正常分支应该用返回值显式表达让调用方知道发生了什么、接下来怎么办。第二类是非预期失败数据库连接断了、事务提交报错、依赖服务超时。这些不该被 catch 后吞掉而应该上抛并触发告警因为它们的发生意味着系统状态可能已经异常需要人工介入。我见过太多团队把这两类混在一起业务判断失败也被 throw 到顶层顶层再统一弹 500或者反过来非预期异常被 catch 住以后假装无事发生。T3 的要求很朴素——在错误路径上代码也必须诚实预期失败告诉调用方具体原因非预期失败告诉运维这里出事了。4.3 测试保护行为契约而不是复读实现T3 的第三根支柱是测试。我先纠正一个常见偏差很多人写的测试其实是实现的复读机——构造函数传了什么参数、内部调用了哪个方法、有几个 mock 都原样照抄。这类测试在重构时最脆弱一改实现就红一片逼得人干脆不重构。我会把测试定位成行为契约的守护者描述给定什么输入、期望什么输出和副作用而不是内部应该怎么调用。比如支付回调最该守护的行为不是回调函数调用了 update 方法而是同一笔订单重复回调不会重复加余额。行为层级的测试稳住了重构起来才有底气——因为它们测的是你不想改变的东西而不是你想重构的东西。5. 实战演示把一段支付回调代码从 T1 推到 T35.1 原始版本先看一坨标准的 T1我们把第一节事故里的代码简化一下就是下面这样非常典型的 T1async function callback(req: any, res: any) { const body req.body const sign req.headers[x-sign] if (verifySign(sign, body)) { const order await OrderModel.findOne({ id: body.orderId }) if (order order.status pending) { if (body.result success) { await OrderModel.updateById(order.id, { status: paid }) await UserModel.incBalance(order.userId, order.amount) res.json({ code: 0 }) } else { await OrderModel.updateById(order.id, { status: failed }) res.json({ code: 0 }) } } else { res.json({ code: 0 }) } } else { res.status(403).json({ code: 1, msg: bad sign }) } }这版本的问题一眼能数出五六个req.body 和 header 全是 any没有解析校验四层 if 嵌套读起来要数括号订单不存在时会返回成功——这是最要命的一处渠道方会以为我们已经受理实际什么都没发生标记已支付和加余额是两条命令中间一旦失败就留下已支付但没到账的中间态重复回调虽然因为状态判断不会二次加钱但对部分失败没有补偿整段代码零日志、零测试。5.2 第一轮重构先让它到 T2重构的第一步我从来不碰类型和测试先做结构。把嵌套的 if 改成早退式的线性流程把加余额的核心逻辑抽成独立函数给回调 body 一个名字清晰的类型别名type CallbackBody { orderId: string result: string paidAt: string transactionId: string } async function paymentCallback(req: Request, res: Response) { const body req.body as CallbackBody const sign req.headers[x-sign] || if (!verifySign(sign, body)) { return res.status(403).json({ code: 1, msg: invalid sign }) } const order await OrderModel.findByOrderId(body.orderId) if (!order) { return res.status(404).json({ code: 1, msg: order not found }) } if (order.status ! pending) { return res.json({ code: 0 }) } await applyPaymentResult(order, body) return res.json({ code: 0 }) } async function applyPaymentResult(order: Order, body: CallbackBody) { if (body.result success) { await OrderModel.markPaid(order.id, body.paidAt, body.transactionId) await UserModel.incBalance(order.userId, order.amount) } else { await OrderModel.markFailed(order.id) } }相比原始版本这个版本能读多了每一条失败路径都有明确的返回新同事读完不需要问订单不存在会怎样。但仔细看它还停留在 T2——body 是断言出来的而不是解析出来的body.result 依然可以是任意字符串订单更新和加余额仍然不是一个原子操作没有测试。这些正是往 T3 推进要解决的问题。5.3 再进一步类型、事务、幂等和测试一起上T3 这轮我分四步走。第一步入口把 unknown 解析成精确类型。写一个parseCallbackBody把 result 限制成success | failed联合类型其他字段逐项校验非法输入直接返回 400type PaymentResult success | failed type OrderStatus pending | paid | failed | closed type CallbackBody { orderId: string result: PaymentResult paidAt: string transactionId: string } function parseCallbackBody(raw: unknown): CallbackBody | null { if (!raw || typeof raw ! object) return null const obj raw as Recordstring, unknown if (typeof obj.orderId ! string) return null if (obj.result ! success obj.result ! failed) return null if (typeof obj.paidAt ! string) return null if (typeof obj.transactionId ! string) return null return { orderId: obj.orderId, result: obj.result, paidAt: obj.paidAt, transactionId: obj.transactionId, } }第二步状态变更和加余额放进同一个事务保证要么都成、要么都败绝不留下中间态。同时更新订单时检查影响行数并发下谁先成功谁生效后到的直接按幂等处理async function applyPayment(body: CallbackBody, order: Order) { const tx await db.beginTransaction() try { const updated body.result success ? await tx.updateOrderStatus(order.id, paid, body.paidAt, body.transactionId) : await tx.updateOrderStatus(order.id, failed) if (updated ! 0 body.result success) { await tx.incrementBalance(order.userId, order.amount) } await tx.commit() } catch (err) { await tx.rollback() throw err } }第三步把回调主流程改成结果对象返回让几种预期失败和成功都被显式表达。顺带一提这个版本里验签对着原始报文 rawBody 做而不是对解析后的对象做——很多团队在这里栽过跟头签名是基于原始字符串计算的字段重新组装一遍再验字段顺序或者空白差异就会让验签失败这类问题排查起来极其痛苦type CallbackOutcome | { kind: success } | { kind: duplicate } | { kind: bad_request } | { kind: invalid_sign } | { kind: order_not_found } async function handlePaymentCallback(rawBody: unknown, sign: string): PromiseCallbackOutcome { const body parseCallbackBody(rawBody) if (!body) return { kind: bad_request } if (!verifySign(sign, rawBody)) return { kind: invalid_sign } const order await OrderModel.findByOrderId(body.orderId) if (!order) return { kind: order_not_found } if (order.status ! pending) return { kind: duplicate } await applyPayment(body, order) return { kind: success } }第四步补测试。最值得守的行为是两个同一笔订单重复回调不会加两次余额、订单更新失败时余额不会被改动。我用伪代码说明测试意图实际写的时候按你们团队的测试框架落地test(重复回调只加一次余额, async () { const body successBody(order-1) await handlePaymentCallback(body, valid-sign) const before await getBalance(user-1) await handlePaymentCallback(body, valid-sign) const after await getBalance(user-1) expect(after).toBe(before) }) test(事务内订单更新失败时余额保持不变, async () { mockUpdateOrderStatusOnce(order-1, 0) // 模拟并发竞争更新行数为 0 await handlePaymentCallback(successBody(order-1), valid-sign) expect(await getBalance(user-1)).toBe(0) })走到这一步这段代码才算真正到了 T3非法输入进不了类型系统部分失败不会留下脏数据重复回调有明确出口关键行为有测试锁住。后面再做任何需求变更回归压力会小非常多。6. 落地心得30 秒给代码分层的体检清单6.1 贴在显示器旁边的六连问框架再好落地靠习惯。我把自己评审时最常用的几个问题压缩成一份 30 秒体检清单现在也贴在我们组里这个函数我能在 60 秒内对新人讲清楚吗讲不清楚就是 T1。除了快乐路径代码里还有几条显式的失败路径一条都没有说明风险全埋在运气里。外部进来的数据有没有统一的解析和校验口子还是全靠运行时才知道拼没拼错字段哪个分支会静默失败静默失败是 T1 最危险的信号之一。改一个业务字段要动几个文件超过三个就要警惕模块边界了。关键业务逻辑有测试吗没有测试的 T2重构时一样不敢动手。这份清单不需要背评审时按顺序过一遍大概 30 秒就能给一段代码打出层级。我后来发现一个规律能快速给出这段 T1、那段 T2结论的瞬间就是团队评审质量开始提升的瞬间。6.2 评审和重构时怎么执行执行层面分享几条实战打法。评审不要追求一步到 T3步子太大会把每个人都搞累。我的做法是先挡 T1 出门明显只有快乐路径、数据不校验、失败被吞掉这些直接在评审会上打回补齐。然后针对模块的重要性做区分跟钱、权限、用户数据相关的核心路径要求 T3普通业务胶水代码到 T2 就放行——胶水代码过度设计维护成本常常比收益还大。重构的顺序也很重要我习惯由外向内先理清出入口把输入解析、错误返回、副作用隔离这三件事定下来再动内部逻辑。入口不动内部怎么抽都是空中楼阁。别一上来就引一堆架构模式先把函数从四层 if 改成一排 return收益立竿见影。6.3 最后说点我自己的体会这套框架用到现在最让我感慨的不是代码质量的提升而是评审氛围变了。以前一句这段写得不好很容易引发争论因为标准不明说的人和被说的人各自有一套评判体系。现在大家会说这段还停在 T1主要缺幂等问题具体、标准统一讨论就从面子问题回到了技术问题。如果你正在带团队不妨从下个评审日开始尝试把我用 T1、T2、T3 给这段代码定个级这句话说出口。你可能会发现第一周大家还有些别扭一个月之后再回头看那些曾被放行的 T1 代码会有一种后怕。把评级的动作变成日常比任何规范文档都管用。
RELATED READING

延伸阅读

更多一线实战笔记与深度复盘,助您持续精进