在一个项目组里面，经常会互相review同事的代码，review的过程中相信会产生不少矛盾，这篇文章记录一下我对代码review的一些看法。首先是编码规范的问题，拿PHP来说。比如命名规范，indent之类的。这些都应该在项目编码开始之前大家立好规矩。是采用驼峰命名法还是用下划线分割单词，indent�是用tab还是用空格，大括号是换行:
function foo() {
}
还是不换行：
function foo()
{
}
这些最基础格式规范一定要在最开始写在一个wiki文档上，项目组所有成员都需要熟记这个规范。这种规范，两上个技术骨干定一下就好了，定下来就按它执行。这个东西其实只要统一就好，无需纠结，孰好孰坏也并不影响项目质量。
接下来我说一下我对关于一些实现逻辑的写法review的个人理解，比较经典的是关于 if...else...�的用法的讨论。如果一个函数中用if...else来做分支处理，代码如下：
if($flag = 1)
{
.....
$result = 'A';
}else if($flag = 2) {
.....
.....
$result = 'B';
}else {
.....
.....
$result = 'C';
}
.....
.....
return $result;
这种情况，如果if...else语句下面的处理跟逻辑跟结果A或者B没有什么关系的话 ，可以改写成如下代码，使得代码逻辑更加清晰
if($flag = 1)
{
.....
return 'A';
}
if($flag = 2) {
.....
.....
return 'B';
}
.....
.....
.....
.....
return 'C';
简单一句话就是，适当的去掉if...else而换成if return结构，很多时候可以提高程序的可读性，但也不能认准这个死道理，比如下面这段（形式A）代码
function foo($flag) {
if($flag) {
$var = 'A';
}else {
$var = 'B';
}
....
}
我的同事review的时候建议改成如下（形式B）：
function foo($flag) {
$var = 'B';
if($flag) {
$var = 'A';
}
....
}
这其实并没有意义，首先这并没有增加程序的可读性，其次形式A的代码其实更符合人类的思维，它表述出$var变量非A既B的逻辑，而下面的代码表述的是$var变量的默认值是A，当flag为true的时候它被赋值B，给人一种它可能还会有其他情况被赋值为C、D ... 之类的值的预期。
所以在review代码的时候不能教条（简单的认为只要去掉else代码就会变得更简单易读），review需要关注的点应该是是否有逻辑漏洞，潜在BUG，非常小可能性出现的异常没有处理。而对于代码的表述方式，不应该过多关注。所以反之若是有人写了形式B的代码，也没有必要在review的时候，让他改成形式A。如果review的这么仔细，不但不会提高代码质量，反而会大大减慢项目的开发进度，增加沟通成本。
所以，review这件事，要有度！
