十年匠心定制 · 商业建站与技术教学双线并行 咨询热线:400-886-1026 service@lmnt.cn
ARTICLE DETAIL

资讯详情

深耕网站建设与运营推广的一线实战洞察。

第331篇 代码审查——怎么review别人的代码

第331篇 代码审查——怎么review别人的代码 上篇聊了测试体系的五个层次从单元测试到实车测试。测试能验证代码的正确性但有件事测试做不到保证代码的可读性和可维护性。你接手过一个项目打开一个文件满屏的a1、tmp2、doSomething3()没有任何注释函数体两百行嵌套五层if-else。代码能跑但你想骂人。这就是缺乏代码审查的后果——每个人只管自己写得爽没人关心整体质量。代码审查Code Review是开发团队保证代码质量最重要的一道防线。它不是走形式、不是浪费时间而是团队工程成熟度的标志。面试的时候几乎每个面试官都会问你们团队怎么做代码审查回答得好说明你有团队协作经验回答不好面试官会怀疑你一直是单打独斗。代码审查到底审什么很多人以为代码审查就是找bug。找bug当然重要但这不是全部。代码审查至少要做四件事逻辑正确性代码逻辑和预期行为一致吗边界条件处理了吗异常路径考虑了吗这是最基本的。设计合理性这个功能放在这个模块里合适吗接口设计是否清晰有没有过度设计或者设计不足好的reviewer会站在架构的角度思考而不只是看眼前的几行代码。比如有人新增了一个全局变量来在两个模块之间传数据reviewer应该指出这破坏了模块的封装性建议通过接口函数或者消息通信来传递。代码可读性变量命名是否清晰函数长度是否合理有没有必要的注释六个月后别人或者你自己能不能看懂这段代码好的代码应该是自文档化的——读代码就像读文档一样顺畅。如果一段代码需要大量注释才能看懂说明代码本身写得不够好。工程规范符合团队的编码规范吗有没有引入不必要的第三方依赖日志打印合理吗错误处理完整吗头文件包含是否有多余的还有一点经常被忽略测试代码也要review。很多人觉得测试代码不重要随便写写就行。但测试代码的质量直接影响测试的有效性。review测试代码时关注测试用例是否覆盖了正常路径和异常路径断言条件是否正确有没有断言了但实际上什么都没验证的情况测试数据是否有代表性机器人项目的代码审查重点机器人项目的代码审查有一些特殊关注点和普通业务代码不太一样。线程安全机器人系统大量使用多线程。传感器数据采集一个线程控制计算一个线程通信又是一个线程。review的时候必须关注共享资源的访问——有没有加锁锁的粒度合理吗有没有死锁风险C里std::mutex用错了就是定时炸弹。实时性约束控制循环里的代码必须在规定时间内执行完毕。review的时候要看有没有动态内存分配new/malloc在实时循环里是禁忌有没有可能阻塞的操作网络IO、文件读写循环体内的计算量是否可控数值稳定性机器人代码大量涉及浮点运算。review的时候要注意除法有没有检查除数为零矩阵运算有没有检查条件数累加操作会不会溢出这些在单元测试里可能测不出来但在长期运行后会积累成问题。资源管理机器人系统通常要长时间运行内存泄漏、文件句柄泄漏、线程泄漏都是致命的。review的时候看new有没有配套的delete智能指针用得对不对线程退出时资源有没有释放。代码审查的常见反模式做了几年代码审查踩过不少坑也见过不少团队的反模式。巨型PR一个PR改了50个文件、3000行代码。reviewer根本看不过来最后象征性扫一眼就批准了。这种PR的代码审查形同虚设。经验法则是一个PR不超过400行改动。如果功能太大拆成多个PR分步提交。LGTM综合征reviewer不看代码直接写LGTMLooks Good To Me。这通常发生在两种情况reviewer太忙没时间看或者reviewer觉得作者水平高不需要看。两种都不对。吹毛求疵reviewer纠结于缩进风格、空行位置这类格式问题忽略了设计层面的问题。格式问题应该交给linter自动处理clang-format、black不要浪费reviewer的精力。审查拖延PR提交后等了两三天才有人review作者等不及就自己合并了。团队应该约定review的响应时间——一般不超过一个工作日。有些团队设立了review轮值制度每天有一个人专门负责及时review。怎么做好一个reviewer做代码审查需要技巧。直接说这代码不行没有任何帮助还会打击同事的积极性。好的review评论应该是这样的这里用了std::vector的动态扩容在控制循环里可能导致内存分配延迟。建议改成std::array或者预分配容量。——指出了具体问题解释了原因给出了解决方案。这个函数有150行建议拆成几个小函数。比如初始化和计算逻辑可以分开。——提出了改进方向但语气是建议而不是命令。这里的锁粒度太大了整个函数都锁住了。其实只有访问共享变量的那几行需要保护可以考虑缩小锁的范围。——具体、可操作。还有个重要的原则对事不对人。review的是代码不是写代码的人。这段代码有问题和你写得有问题意思差不多但感受天差地别。面试追问你们团队用什么工具做代码审查GitHub/GitLab的Pull Request机制。提交PR后指定两到三个reviewer。reviewer在PR上留评论作者修改后回复所有评论resolved后才能合并。CI跑完测试全绿也是合并的前提条件。我们还会用Code Owners文件指定每个目录的负责人涉及该目录的PR自动要求对应owner review。代码审查会不会拖慢开发进度短期看确实会。一个PR从提交到合并可能要等半天到一天。但长期看代码审查减少了后期返工的成本。现在花半小时review比上线后花三天修bug划算得多。而且代码审查有知识传播的效果——新人通过review学习团队的编码规范和设计思路。我们统计过做了代码审查之后线上bug率下降了40%左右。遇到review意见不一致怎么办先讨论讨论不出结果就找第三个资深工程师仲裁。最终决定权在代码作者手里——他要为这段代码负责他有权选择接受或拒绝建议。但如果涉及安全和架构问题团队lead有否决权。关键是要把讨论过程留在PR评论里这样以后有人回头看这段代码时能理解为什么做了这个决定。有没有用过自动化的代码审查工具有。静态分析工具cppcheck、clang-tidy在CI里自动跑检查常见的编码错误和潜在bug。代码格式检查clang-format也是自动的格式不对直接CI失败不需要reviewer操心。还有一些工具可以检查圈复杂度、函数长度、文件行数超过阈值就报警。这些自动化检查把低级问题过滤掉了reviewer可以把精力集中在设计和逻辑层面。代码审查是团队工程文化的体现。一个认真做review的团队代码质量不会差。一个从不review的团队迟早会被技术债压垮。我见过最好的代码审查文化是这样的每个新人都被鼓励review老员工的代码从中学习设计思路和编码技巧。不是走形式而是真的认真看、提问题。老员工会耐心解释为什么这样设计也会承认某些地方确实可以改进。这种氛围下代码审查不是负担而是团队学习和成长的方式。面试时聊聊你的review经验——你review时关注什么、你怎么给反馈、你怎么处理分歧——这些比我会review三个字有说服力得多。下一篇聊版本管理。代码审查保证每次合入的代码质量版本管理保证你能追溯每一次变更的历史。Git在机器人项目中的使用有一些特殊之处——大文件管理、多仓库协调、发布分支策略——这些在面试中也经常被提到。如果这篇文章对你有帮助欢迎点赞、在看、转发三连。 你的支持是我持续更新的最大动力。「机器人软件开发面试·从入门到精通」连载系列上一篇第330篇 硬件在环测试——虚实结合的HIL验证下一篇预告第332篇 版本管理——Git在机器人项目中的最佳实践有任何问题欢迎评论区留言我会尽量回复。
返回列表