或许应该说,Code Review, 达到了它的目的了吗?
最近在读一本比较牛逼的书,Desiginning Data-Intensive Applications: The Big Ideas Behind Reliable, Scalable, and Maintainable Systems. 正如书的题目,一个牛逼的软件,需要有3个重要元素:Reliability, Scalability, Maintainability. 今天所说Code Review与Maintainability。
可维护性,主要指,可操作性,简洁性,可扩展性。程序设计的最重要的目的,就是降低复杂度,还有易于扩展。如何降低复杂度?如何编写易于扩展的代码?在复杂的工程中如何保证程序设计的优雅?
简洁,依赖于良好的分层和抽象;可扩展,依赖于对需求和可能变化的深入理解和重构。如题目所说,code review,是很多公司在充分实践的一种方法。
在往常经验里,无论是review别人的代码,还是自己提的review,都会有一些共有的问题:
- code review的工具不一定能够像我们在IDE里面读代码一样支持语义上的跳转去帮助理解,导致review的碎片化,对于reviewer,无法形成一个处理流和整体。
- 难以对代码涉及到的架构设计、对整个系统的性能影响有全局的思考和审视,review忽视系统,陷入细节。
- 过多的业务细节和实现细节无法理解。
Code Review很多时候退化成了静态代码分析和自动化测试能够做到的:代码风格检查,局部bug的发现....,比如:函数名/变量名命名较差,更好的算法实现,数据结构实现...代码bug... 。距离在正确达到意图的前提下保证降低复杂度和提高代码可扩展性的目标有一些距离,
在我刚来到头条的时候,我也惊讶于一个工程师的自由度很高,独当一面,测试后上线,开发、迭代非常快。也曾一度向我的主管讨论过code review的必要性。 到了今天,我反而觉得,code review重要,但是被overrated了。
虽然有code review,代码仍然可能走向一团糟,熵增大,难以维护。
一个不够好的架构与设计下,再多的review和修补也无法降低系统的熵...
需求迭代加快的情形下,在一个不以技术驱动的团队,难免会走在破窗效应的路上,即使有了review,缺乏系统思考的时候,破坏设计原则的修改可能被通过...
甚至,我觉得,你周围最优秀工程师的水平上限就是你的上限。
一个有经验的工程师都知道,程序设计最耗费脑细胞和时间的可能不是编写代码,而是设计,去在思维的palace里构建出解决问题的模型和思路。对于构建系统来说,也是一样的。
花费一个团队最多时间的不应该是对test-ready的成品的审查,而是对系统的架构设计、实现/重构方案的讨论与Interface Review。
在技术驱动的团队里,即使需求很紧急,对于关键的功能,核心成员也会耐心地审视架构设计和实现方案,如何控制熵,如何用更合理的方式实现,如何考虑到未来的变化。
技术驱动的团队里,应该持续进行对设计的调整和代码的微小重构与改良,时刻在整个系统的设计和表现(performance)角度审视自己的工作。这也是“系统思考”的核心。
大部分的代码的熵增大难以控制,并不是因为没有好的架构,而是因为迭代中忽略了系统性的思考和审视,而是用局部的解决方案解决问题,在反复迭代后,复杂度过高导致控制熵变得异常困难。这是code review比较难解决的。
除了团队增加对Architecture Design和Interface Review的重视外,保证团队成员的优秀也是非常重要的。一个优秀的工程师需要能够独当一面,能够在系统角度实现局部的良好设计,通过合理的测试方法论验证结果。能够用合理的数据结构、算法实现功能。我想,是一个优秀工程师的基本功。
以上的实践,在我们的团队很常见,我只是其中一员。
也是我司招人的原则:与优秀的人共事。招人,就招优秀的人。
另外了,大部分的代码风格问题,实现bug问题,性能问题,不一定要通过code review,而是有其他思路的:
- CI中集成静态代码检查工具,发现潜在的bug和较差的代码风格问题。
- 工程师具备设计测试case,考虑全面细致的能力。
- 系统的可操作性,也就是说,具备足够的监控和手段去观察它的健康状态,性能,通过积累的性能分析数据去针对性优化。(切忌过早优化)
- 自动化集成测试,避免人类的错误(很常见)
另外,Facebook经常发布Bug很多的代码,却并没有产出很多的线上事故的一个重要原因是,具备非常完备的灰度系统和监控设施,在能够精确控制影响范围的前提下,及时发现问题,在更多人被影响之前修复问题。这也是一种很好的思路。
技术驱动的团队切忌把方法固定成流程,最后趋于表面的形式,忘记了初心,忘记了最重要的东西:降低复杂度(简洁性),易于扩展。