代码审查查什么?

不应该做的

  • 格式:在什么地方放置空格和断行符?使用制表符还是空格?大括号如何放置?
  • 风格:变量、参数是否声明为final?函数变量的定义是否与调用代码或函数开始代码太相似?
  • 命名:域、常量、变量、参数、类的名称是否符合标准?命名是否太短?
  • 覆盖测试:这段代码是否进行了测试?

应该做的

基本原则

设计

  • 新代码是否与整体架构匹配?
  • 代码是否遵循SOLID原则领域驱动设计以及团队喜爱的其它设计模式。
  • 新代码采用哪些设计模式?这些模式合适吗?
  • 如果代码库采用混合标准或设计风格,新代码是否符合当前风格?代码迁移是按正确的方向进行,还是效仿即将被淘汰的陈旧代码?
  • 代码是否处于正确的位置?例如,如果代码执行与顺序有关,它是否能按顺序执行?
  • 新代码能否复用部分已存在代码?新代码能否给已存在代码提供复用部分?
  • 新代码是否包含冗余代码?如果包含,是应该重构成更加可复用部分,还是在这个阶段能接受这种冗余?
  • 代码是否超出设计标准?这种复用构造现在是不是不需要?团队如何根据YAGNI权衡复用?

可读性和可维护性

  • 命名(字段、变量、参数、函数以及类)能否反映它们代表的事物?
  • 通过阅读代码,我能否理解代码的目的?
  • 我能否理解测试的目的?
  • 测试是否覆盖了绝大部分情况?是否覆盖常见情况和异常情况?是否存在没考虑到的情况?
  • 异常错误消息是否易于理解?
  • 难以理解的代码段是否进行了备注、评论或者使用易懂的测试案例进行覆盖(符合团队的偏好)?

功能

  • 代码的实际工作是否符合预期?如果有自动化测试来确保代码的正确,测试能否测出代码满足约定要求?
  • 代码看上去是否含有细微错误,比如使用错误变量进行检查,或者把 or 误用为 and?

其他

  • 代码中是否存在潜在的安全问题?
  • 是否需要满足规范要求?
  • 对于没有覆盖自动化性能测试的代码段,新代码是否引入了不可避免的性能问题,比如不必要的数据调用或远程服务?
  • 作者是否需要创建公共文档,或者修改现有的帮助文档。
  • 展示给用户的消息是否检查无误?
  • 是否存在导致产品崩溃的明显错误?代码是否会意外指向测试数据库,或者是否有应该替换成真正服务的硬编码存根代码?

测试

  • 新的或修改的代码有测试吗?
  • 测试有覆盖到代码中令人困惑或者复杂的部分吗?
  • 我能理解这些测试么?
  • 这些测试需要满足什么要求?
  • 我可以考虑没有被现有测试覆盖到的用例吗?
  • 这些测试是否有说明代码的限制条件?
  • 审查代码中的测试类型、测试级别正确吗?
  • 有没有针对安全性的测试?
  • 审查者也可以写测试

性能

  • 这部分功能有硬性的性能需求吗?
  • 如果有,有测试去检查吗?
  • 功能修改或新功能对已有性能测试结果有不良影响吗?
  • 代码审查中没有硬性的性能需求要怎么做?
  • 数据库调用是否影响性能?
  • 是否有不必要的网络调用?
  • 移动程序、可穿戴程序是否过于频繁地调用后端?
  • 代码通过锁来访问共享代码吗?这样会导致性能下降和死锁吗?
  • 代码会造成内存泄漏吗?
  • 应用程序的内存会无限增加吗?
  • 代码有关闭连接或流吗?
  • 资源池配置正确吗?
  • 如果你检查包含反射的代码,问一下反射是不是一定需要?
  • 如果超时,系统中的其他部分会受到什么影响?
  • 代码中有使用多个线程来执行一个简单的操作吗?除了增加时间和复杂性外,却没有带来性能的提高?代码使用并行流机制,但是却没有从并行性中受益吗?
  • 多线程环境下的代码使用了正确的数据结构吗?
  • 代码容易出现竞态条件吗?
  • 代码中对锁的使用正确吗?
  • 代码的性能测试有价值吗?
  • 如果审查的代码使用了缓存,不正确的缓存项失效是否进行处理?

代码级优化

  • 代码中是否使用了不必要的同步、锁?如果代码总是执行在单线程下,加锁只会带来额外开销。
  • 代码中是否使用了不必要的线程安全数据结构?例如,可以用ArrayList 替换 Vector 吗?
  • 代码中是否使用了在常见操作上性能很差的数据结构?例如,使用了链表结构,却经常搜索其中的一项。
  • 代码是否使用了锁或者同步机制,而实际上可以用原子变量替代?
  • 代码可以得益于延迟加载吗?
  • if 语句或其他逻辑语句能通过短路机制进行优化吗?比如把最快的计算放在条件的开始?
  • 有很多的字符串格式吗?可以更有效率吗?
  • log 语句有使用字符串格式化吗?有用检查log级别的 if 语句包起来或者使用的日志提供者可以进行延迟操作?

数据结构

  • 数据结构是否被正确的使用?
  • 是否使用了反模式

SOLID原则

SOLID代表:

  • S – 单一职责原则:不应该有多种情况需要修改某个类的对象
  • O – 开放封闭原则:软件实体应该对扩展开放,对修改封闭
  • L – 里氏替换原则:函数使用基类的引用,必须能够在不知不觉的情况下使用派生类的对象
  • I – 接口分离原则:多个客户端特定的接口比使用一个通用的接口要好
  • D – 依赖倒置原则:依赖抽象。不要依赖于具体实现

一些代码异味可能意味着可能已经违反了一个或多个SOLID原则:

  • 很长的if/else语句
  • 强制转换成一个子类型
  • 很多公共方法
  • 实现的方法抛出UnsupportedOperationException异常

安全

  • 了解第三方库
  • 检查是否需要对新路径和服务进行身份验证?
  • 您的数据是否需要加密?
  • 密码、加密密钥、token等秘密数据是否被正确管理?
  • 代码应该记录/审计行为吗?这样做是否正确?
    • 代码是否会进行任何数据更改(例如添加/更新/删除)?它是否应该记录所做的更改,由谁以及何时进行?
    • 此代码是否在某些性能关键路径上?是否应该在某种性能监控系统中记录启动时间和结束时间?
    • 任何记录的消息的日志记录级别是否合适?

参考资料

©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 212,185评论 6 493
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 90,445评论 3 385
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 157,684评论 0 348
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 56,564评论 1 284
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 65,681评论 6 386
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 49,874评论 1 290
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 39,025评论 3 408
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 37,761评论 0 268
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 44,217评论 1 303
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 36,545评论 2 327
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 38,694评论 1 341
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 34,351评论 4 332
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 39,988评论 3 315
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 30,778评论 0 21
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 32,007评论 1 266
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 46,427评论 2 360
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 43,580评论 2 349

推荐阅读更多精彩内容

  • 目标:1、希望孩子拥有更多朋友更加开心快乐!智慧多多!不计较!不抱怨! 2、不紧张,不焦虑,随时提醒自己是谁,是谁...
    zl向日葵阅读 217评论 0 0
  • 1.Promise的含义:是异步编程的一种解决方案,比传统的解决方案--回调函数和事件--更合理和更强大。ES6统...
    ningluo阅读 298评论 0 0
  • 01/ 我听过很多的爱情故事,仿佛大多都需要一个懂套路的人主动些到最后自然而然的在一起。 也写过很多很多的故事,但...
    北栀一刺阅读 965评论 2 2
  • 爱似水仙,很喜欢的一首歌,确切的说是喜欢金海心唱的。不知道什么时候第一次听,但是后来才突然间喜欢了。以至于...
    shenghuoshi阅读 172评论 0 1
  • 他走了,一句话没说的就走了,明明早上送早饭的时候还吃了一大碗稀饭,精神还特别的好,中午还吃了一碗肉饼汤一大碗饭。所...
    玉米大棒槌阅读 159评论 0 0