TDD 代码评审点分类总结

admin · March 08, 2021 · 9 hits

作者:kaelzhang
出处:https://mp.weixin.qq.com/s/aenPtdenZBdA0Nmj1NvKWg
版权:本文版权归作者所有,如需认领,请联系管理员

根据《Google 代码评审应该看什么》这篇文章,经过实践对代码评审点进行了分类总结:

设计层面

功能性

此 CL 是否达成开发人员的预期目的?开发人员打算为该代码的 “用户” 带来什么价值?“用户” 通常既是最终用户(当他们受到变更影响时)又是开发人员(他们将来不得不 “使用” 该代码)。

通常,我们希望开发人员能够对 CL 进行充分测试,以确保它们在进行代码审查时能够正常工作。但是作为评审者,你仍然应该通过阅读代码考虑边界情况、寻找并发问题、尝试像用户一样思考,确保没有任何错误。

对于评审者来说,可以根据需要检查 CL。而检查 CL 的行为最关键的时间应该花费在对用户的影响(如 UI 变更)上。当仅阅读代码时,很难理解某些变更将如何影响用户。对这类变更,如果评审者不便在 CL 中 patch 并亲自尝试,则可以让开发人员向你演示该功能。

在代码评审中需要特别考虑功能性另一个场景是:在 CL 中是否正在进行并行编程,从理论上讲这可能会导致死锁或竞态条件。仅通过运行代码就很难检测到这类问题,通常需要有人(开发人员和评审者)仔细考虑它们,以确保不会引入问题。(请注意,这也是可能出现竞态条件或死锁的情况下,不使用并发模型的一个好的理由,这会使代码评审或理解变得更为复杂。)

设计

评审中最重要的内容是 CL 的总体设计。CL 中各代码片段之间是否有意义?此变更是属于代码库还是属于基础库?它与系统的其余部分是否集成良好?现在是添加此功能的时候吗?

实现层面

一致性

一致性对于代码可维护性非常重要,这也是为什么要统一语言达成共识的原因、制定共同的代码规范等实践的根本原因。该 CL 的变更内容自身是否是一致的?该 CL 的变更内容是否与之前已有的内容是否是一致的?包括:代码写法的一致性(例如:字符串为空,是判断长度为 0 还是与空字符串比较)、概念的一致性(例如:工具概念是使用 tool 还是 util)、缩略语使用的一致性(例如:使用 rsp 还是 resp)等。

复杂性

该 CL 是否比它所应该的实现要复杂?在 CL 的每个级别都进行检查:个别代码行是否太复杂?功能是否太复杂?类是否太复杂?“太复杂” 通常表示 “通过代码阅读无法快速理解。” 这也可能意味着 “开发人员在尝试调用或修改此代码时可能会引入错误。”

复杂性的一种特殊类型是过度设计,即开发人员使代码变得比实际需要更为通用,或者增加了系统目前不需要的功能。评审者应特别警惕过度设计。鼓励开发人员解决他们现在需要解决的已知问题,而不是解决推测将来可能需要解决的问题。将来的问题应该当它出现时再解决,那时你就能在真实世界中看到它的实际的样子和要求。

命名

开发人员是否为所有事物都起了好的名字?一个足够长的好名字可以充分表达事物的含义或作用,而又不会过长而难以阅读。

测试

根据变更进行合理的单元测试、集成测试或端到端测试。通常,除非 CL 处理紧急情况,否则应在与生产代码相同的 CL 中添加测试。

确保 CL 中的测试正确,合理且有用。测试不会自我验证,我们很少为测试编写测试 - 所以必须确保测试的有效性。

如果代码有问题,测试会失败吗?如果代码进行修改,测试将会产生误报吗?每个测试都会做出简单而有效的断言吗?测试是否在不同的测试方法之间适当地分离?

请记住,测试代码也是必须维护的代码。不要仅仅因为它们不是制品的一部分而容忍测试代码中的复杂性。

规范层面

注释

开发人员是否写下了清晰、可理解的注释?所有注释实际上都是必要的吗?通常有效的注释是:解释了为什么存在某些代码,而不应解释代码在做什么。如果代码不够清楚,无法做到自解释,则应使代码变得更简单。当然也有一些例外情况(例如,正则表达式和复杂算法通常会从注释中解释它们的作用而受益),但注释大多是针对代码本身可能无法包含的信息,例如:背后的决策原因。

查看此 CL 之前存在的注释也可能会有所帮助。也许有一个待办事项现在可以删除,建议不要进行此变更的注释等。

请注意,注释与类,模块或函数的文档不同,注释应描述一段代码的目的,应如何使用这段代码以及代码被使用时的行为。

风格

直接参考 Google 提供编程语言的风格指南。

参考文献

  1. https://google.github.io/eng-practices/review/reviewer/looking-for.html

「软件匠艺社区」旨在传播匠艺精神,通过分享好的「工作方式」,让帮助程序员更加快乐高效地编程!

No Reply at the moment.
You need to Sign in before reply, if you don't have an account, please Sign up first.