如何进行CodeView

关于Code Review的重要性，我相信好的工程师都能认识到。 参考 "让Code Review称为一种习惯" 和 "从Code Review谈如何做技术"。

同时引用一下有人对Google Code Review的描述：

The biggest thing that makes Google’s code so good is simple: code review. At Google, no code, for any product, for any project, gets checked in until it gets a positive review.

Code Review 主要Revivew什么

Architecture/Design

单一职责原则.

这是经常被违背的原则。一个类只能干一个事情, 一个方法最好也只干一件事情。 比较常见的违背是处理促销的时候，同时处理验券逻辑。

行为是否统一

比如缓存是否统一，错误处理是否统一， 日志打印是否统一， 方法实现是否统一等等。

同一逻辑/同一行为 有没有走同一Code Path？低质量程序的另一个特征是，同一行为/同一逻辑，因为出现在不同的地方或者被不同的方式触发，没有走同一Code Path 或者各处有一份copy的实现， 导致非常难以维护。

代码污染

使用魔数，例如直接比对数字。编写不容易阅读的代码，应用了很多复杂的设计。

重复代码

主要看有没有把公用组件，可复用的代码，函数抽取出来。

Open/Closed 原则

就是好不好扩展。 Open for extension, closed for modification.（做好接口设计会很好解决这些问题）。

面向接口编程 和 不是 面向实现编程

主要就是看有没有进行合适的抽象， 把一些行为抽象为接口。

健壮性

对Corner case有没有考虑完整，逻辑是否健壮？有没有潜在的bug？

有没有内存泄漏？有没有循环依赖?（针对特定语言，比如Objective-C) ？有没有野指针？

有没有考虑线程安全性， 数据访问的一致性

错误处理

有没有很好的Error Handling？比如Price接口出错。

有没有增加必要的监控，及时报警。

改动是不是对代码的提升

新的改动是打补丁，让代码质量继续恶化，还是对代码质量做了修复？

效率/性能

两层循环，数据请求量的限制，较大数据等耗时操作是否处理得当。

关键算法的时间复杂度多少？有没有可能有潜在的性能瓶颈。

复杂需求，是否有必要的设计， 可预见的效率问题， 开发模式一致性的问题 应该尽早在Design Review阶段解决。如果Design阶段没有解决，那至少在Code Review阶段也要把它找出来。

Style

可读性

衡量可读性的可以有很好实践的标准，就是Reviewer能否非常容易的理解这个代码。 如果不是，那意味着代码的可读性要进行改进。

命名对可读性非常重要，我倾向于函数名/方法名长一点都没关系，必须是能自我阐述的。

英语用词尽量准确一点（哪怕有时候需要借助Google Translate，是值得的）

函数长度/类长度

函数太长的不好阅读。 类太长了，比如超过了1000行，那你要看一下是否违反的“单一职责” 原则。

恰到好处的注释。 但更多我看到比较差质量的工程的一个特点是缺少注释。

参数个数（不要超过5个参数）

Review Your Own Code First

跟著名的橡皮鸭调试法（Rubber Duck Debugging）一样，每次提交前整体把自己的代码过一遍非常有帮助，尤其是看看有没有犯低级错误。

如何进行Code Review

多问问题。多问 “这块儿是怎么工作的？” “如果有XXX case，你这个怎么处理？”

每次提交的代码不要太多，最好不要超过1000行，否则review起来效率会非常低。

当面讨论代替Comments。 大部分情况下小组内的同事是坐在一起的，face to face的 code review是非常有效的。

区分重点，不要舍本逐末。 优先抓住 设计，可读性，健壮性等重点问题。

Code Review的意识

作为一个Developer , 不仅要Deliver working code, 还要Deliver maintainable code.

必要时进行重构，随着项目的迭代，在计划新增功能的同时，开发要主动计划重构的工作项。

开放的心态，虚心接受大家的Review Comments。

审核人及职责分配
所谓职责归属：即指谁对这个Code负责

提交人

审核人

职责归属

P2.1及以下

P2.3

审核人

P2.2及以上

P2.3

提交人&审核人

p2.3及以上

p3.1/p3.2

提交人&审核人

对于comments的检查。周会上抽查。

审核流程
开始
RD完成开发自测
自查不过
自查通过
Review your
own code
RD按照comment意见修改
重新自测并重新提交代码
是
否
是否可以Merge
Merge至主分支
1.只有主分支权限的人才能Merge

结束
是
否
是否需要
代码走读
RD发起Code Review
跟著名的橡皮鸭调试法（Rubber Duck Debugging）一样，每次提交前整体把自己的代码过一遍非常有帮助，尤其是看看有没有犯低级错误。
是否走读标准
1.修改了核心链路代码；
2.修改代码行数> 600 行；
3.修改公用业务组件或逻辑；
4.审核人提出走读意见；
1.按照要求选择审核人；
2.设置PR完成时间；
3.补充code主要变动范围
4.跟进PR完成状态
发起人组织
Code Review会
stash上同步记录 commits
否
Review
是否通过
merge条件：
1.approve >= 2人；
2.comment 都完成修改；

备注标签
流程节点
分支判断
图示例

常见代码开发规范

thrift服务提供一般情况都需要在finally里输出info级的入参和返回参数的日志

实现Object的toString方法而不是使用json序列化工具输出日志

Integer的等值使用equals而不是==

使用apache commons 包而不是自己造轮子

组织参数抽取方法来完成而不要在service里面写，方法命名规范成buildXXXXX

线程池的使用需要统一到一个Utils类中，避免线程池定义泛滥。

所有远程服务调用都需要封装delegate类，禁止直接使用octo客户端进行调用。在真正调用一般都要finally里输出info日志 入参和返回值。

静态变量需要统一在一个类中定义。

OCTO生成的类不要传递到service层，构造一个本地类进行使用。

不要把配置写在代码里，充分利用MCC和*.properties

MCC里配置开关和频繁修改的属性，长期不修改的放到*.properties

给方法起名字是为了自己和其他人能看懂

变量命名以及方法名不要使用拼音缩写

Json序列化统一使用travel-insurance-common表里的JsonUtils

不要在ThriftService里写业务逻辑。下沉service

不要拷贝代码

执行update语句一定要注意 where 条件里 一定要走索引，这句话的意思是，把where 条件单独放一个select里 然后explain一下，如果是全表扫描 那这个SQL会锁全表。

不要在返回boolean的方法里面写if (xxxxx&xxxx){return true}else{ return false}

永远不要在事务里rpc

抛出异常最好子类化，这样从cat看到异常就知道是啥原因。

逻辑嵌套不要超过3层

@Transactional(rollbackFor =Exception.class) 使用事务的时候 一般情况这么用。

查询不要使用mybatis生成的condition来拼，mybatis生成工具只负责生成表的po和最基本的insert 和update ，如果有分表，请所有sql全部手写，必须带分表键。

不要在static{}代码块里做初始化动作，禁止在static{}代码块里rpc