code review到底在审查什么?

code review到底在审查什么?

在做一个代码的code review时,以下是我们需要关注的:

是否明晰

  • 代码应该易于阅读,应该一目了然地了解每行和每个功能,否则,它太复杂了。

  • 代码应符合模块内已使用的编码风格(缩进,间距,命名约定等)

  • 每个函数或方法的预期意图都应简洁明了,并且一个函数不能超出一屏(几十行代码)。否则,它太复杂了。

  • 代码是否适当地增加了注释,以供他人理解和维护?

注释是否简洁,易于理解,并且实际上为周围的代码增加了价值?

每个函数的意图是否明确,或者增加描述意图的最小注释是否会有所帮助?

API是否用Javadoc(Java)记录?

解释为什么和解释是什么一样重要,记录意图是最重要的记录形式,因为没有人知道你在想什么。

  • 是否使用了不必要的和/或晦涩的语言技巧,即使仅仅30秒也可能使读者感到困惑?
  • 每个函数是否桥接一个抽象级别,调用较低级别的函数?
  • 是否存在跨越数十行的“函数”功能,这些功能可以轻松拆分为子函数?
  • 代码被注释掉了吗?在极少数情况下,这是适当的。一般来说,注释掉的代码使阅读起来更困难,也很难理解-不要以为每个人都在使用语法突出显示。

正确性

  • 有明显的错误吗?
  • 有单元测试并且已经运行了吗?
  • 单元测试是否足以测试代码(或代码的更改)?是否有代码覆盖工具会自动与构建一起运行?
  • 是否是线程安全的?是否在类或API级别记录了线程安全性和并发性,以便调用者理解预期的语义?

设计意义

  • 设计是否有意义(更改或不更改)?
  • 更改是否适合现有设计(例如,新职责是否属于更改后的实体)?
  • 所做的更改是否揭示了现有设计中的缺陷(例如,简单的行为更改需要复杂的代码更改)?
  • 此更改是否完全解决了所有功能/问题的要求?需求的解释方式可能导致生产中的问题是否存在歧义?

重用和重复代码

  • 包内的代码是否重复?如果在模块中甚至几行代码被重复复制了多次,则表明需要重构。
  • 共享库中应该有局部函数吗?
  • 是否存在可以完全删除或替换为现有库的代码?

配置和常量

  • 是否在整个代码中散布了魔术数字或字符串常量,或更糟糕的是,重复多个地方使用这些常量?
  • 是否有所有配置都设置了默认值?
  • 配置文件是否增加了清晰的配置说明?

向后兼容和版本控制

  • 该代码向后兼容其预先存在的API吗?
  • 代码向后兼容其预先存在的配置吗?
  • 是否存在并非必需的向后不兼容的更改?
  • 如果存在向后不兼容的更改,将如何在软件包构建和部署方面进行管理?

依存关系

  • 是否引入了新的外部依赖关系,是否会带来新的可用性或操作风险?
  • 是否确实需要每个新库依赖项?是否每个现有依赖项都绝对必要?您能以更简单的方式解决问题吗?不要仅仅将最新和最出色的组件视为依赖项,因为您认为它们是浮华的。
  • 是否有替代的“较轻量”依赖项可以替代使用?

故障处理

  • 该代码可处理任何库代码调用返回的意外值?

  • 代码对非法输入和空输入怎么处理?

  • 该代码是否处理了边缘case?

  • 代码是否有清晰一致的异常处理策略?

性能

  • 是否有过早优化的迹象?
  • 有没有明显的性能或效率问题可以解决?
  • 如果大部分额外的时间都花在了代码阻塞上,代码执行时间也会更长吗?

性能指标metrics

  • 该指标是否有助于运营活动?
  • 是否容易知道已部署的代码在做什么?
  • 从错误到调试的所有级别都有适当数量的日志记录吗?
  • 代码中是否收集了性能指标?
  • 每个日志消息是否都包含足够的相关信息,以便某人仅阅读日志消息就能了解系统的行为?
  • 测试中是否包装了复杂的info / debug / verbose日志语句if (log level),以免影响性能?

SQL

  • 用户输入是否用于构建原始SQL?(请参阅SQL注入)
  • 在检查之前是否已计划好所有SQL查询的解释?
  • 是否在每个SQL语句上方以注释的形式签入了解释计划结果,作为开发人员期望查询执行的快照?在比较未来的解释计划以检查假设是否发生变化时,这可能非常有用。
  • 是否正确使用了绑定变量,或者生成了一个关闭的SQL语句?
  • 此更改是否需要回填或直接数据库更新,并且是否已审查了回填脚本?
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 206,126评论 6 481
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 88,254评论 2 382
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 152,445评论 0 341
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 55,185评论 1 278
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 64,178评论 5 371
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 48,970评论 1 284
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 38,276评论 3 399
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 36,927评论 0 259
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 43,400评论 1 300
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 35,883评论 2 323
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 37,997评论 1 333
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 33,646评论 4 322
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 39,213评论 3 307
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 30,204评论 0 19
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 31,423评论 1 260
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 45,423评论 2 352
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 42,722评论 2 345

推荐阅读更多精彩内容

  • 去年有段时间得空,就把谷歌GAE的API权威指南看了一遍,收获颇丰,特别是在自己几乎独立开发了公司的云数据中心之后...
    骑单车的勋爵阅读 20,437评论 0 41
  • 概要 64学时 3.5学分 章节安排 电子商务网站概况 HTML5+CSS3 JavaScript Node 电子...
    阿啊阿吖丁阅读 9,095评论 0 3
  • 今天,我在十字路口等奶奶。一场运动会正不知不觉地开始了。 运动会马上开始了,一名男子!已经等不及了。只见...
    乐乐姐_11db阅读 186评论 0 5
  • 一、基本概念: 注:对于git的分布式概念及其优点,不重复说明,自己百度或谷歌。本文中涉及到指令前面有$的,在cm...
    大厂offer阅读 1,408评论 0 3
  • 以前的牙白小姐,生活在一片绿色的菜地里,牙白小姐从小就很羡慕那些成年了的萝卜青菜们,成年了就会离开这片土地,去向更...
    猫咪先生要觅食阅读 222评论 0 0