到底该怎么样做代码审查？

 

近几天，算是掉进来一个巨大的焦油坑，我和另外三个同事备受折磨。现在三个有一个跑去装机器了，暂时不写代码，还有我和另外一个同事。

其实代码复杂度不是很复杂，但是代码审查（简称CR）就是过不了，来来回回的改，若是去和较真CRer，貌似也理由也不是很充分，添加的CR评论多半是“我觉得”，“我认为”，CR代码的spell check和style check胜过代码的逻辑check。

比如，你一个循环，可能多半人都会这么写：

for(int i=0;i<100;i++)
{
    ......
}

 

但是可能有些人会这么写

 

// The max number is 100
int maxNum=100;

// Add some comments here
for(int num=0;i<maxNum;i++)
{
    .......
}

 当然第二种方式看起来比第一种“优雅”，但是我认为第一种也不为过。

现在没人关心这段代码能不能得到想要的结果，这个只能写code的人去心中有数了。

 可能有人会提到易读性，维护性，我想这个有时候有点小题大做了，代码足够简的情况下，如果还要掰扯个最优写法，吹毛求疵，不为过。

如果写每一段代码都要考虑，这个是不是别人看会不会明白？会不会造成误解？是不是已经足够清晰？

那这个就不是写代码，这个就是写文章了。

我认为代码的易读性跟怎么写不太有关系，而是跟你整个框架的搭建，对行为的抽象精炼有关系。

再举个例子：http://msdn.microsoft.com/en-us/library/ms243496(v=vs.110).aspx

相信大家在写单元测试的时候广泛用到Assert，有这么一个方法。\

 

public static void AreEqual(
    Object expected,
    Object actual,
    string message
)

 

关于第三个message参数，msdn是这么定义的：

 

messageType: System.String
A message to display if the assertion fails. This message can be seen in the unit test results.

 

说白了就是写一个信息，然后在断言失败的时候打印出来。

但是注意，这虽然是一个message of error，但是msdn 没有将message定义为errormessage，errormsg等。也就是说没有给这个message定性，只是说在失败的时候打印一个message。

其实你要是说写一个Assert.AreEqual("Bob", userName, message) 你会怎么写你的message。

如果失败，他会打印出：

 

Expected : "Bob"; Actual :"Boa", message

 

先说我的想法，我是在message写明这个断言的作用，会写“Verify if the user name is 'Bob' or not”, 表明这个断言在验证什么。

但是按照我们现在的写法，他需要写成“ Verify user name  ‘Bob’ fails.”， 区别在于我的是个不确定的语气，而规定是肯定句。

 

其实有必要掰扯message怎么写吗？按照多数人来说，可能觉得微不足道。

我其实两者怎么·写都可以接受，但是就因为我的这种写法，被谈话许久，其实最后的的结论就是看到第二种写法，第三者不用看code就知道哪里的功能错了。

这个理由仍然不能让我信服，既然你能看到message，说明断言已经失败了，继而看到这个断言在验证什么也可以推知错误所在。

最后我把messge给删除了，就光verify，不显示message，也不错吧。

但是人在屋檐下，怎么着我都是能接受的，你让我怎么写我就怎么写，你制定出个rule出来，我follow就好了，不是吗？

这就好比我不懂OO，我反OO难道我就不能写出好的code吗？不见得（我且认为满口OO的人，才是最不懂OO的人）

=================================================================================

代码如文章，现在几个人写一本书也不可能都文风一致。

现在每天最头疼的，不是功能如何事先，而是纠结

如何给一个变量起个名字，现在是能不用变量就不用变量，函数里套函数

如何添加注释，现在是能不写就不写，写了 反倒多余

代码空格对齐问题   哪里少了1个空格，哪里哪里要对齐。。。。。。

新function能不写就不写，堆代码，写了有人会comment

蛋疼的事情一大堆

。。。。。。。。。。。。。。。。。。。。。。。。。。。

新的代码怎么放置，添加一个文件如何命名，一个变量放在哪个文件里都需要请示。

====================================================================================

 关于代码风格，我认为只要不是堆砌，只要风格简洁，思维通顺，就为好的。而且你既然作为一个程序人员，基本的阅读别人代码的能力是必须的。

好的代码能让人明白你的思维，你能明白你当时的设计思路。

跟一同事飞总说了两句，说了句经典的：他们看不懂逻辑，所以就只能挑code style和format的错。

 

啰嗦完毕，吃饭

也请大家谈谈自己的意见，该怎么做CR经济又实惠？

先谢谢大家

评论

2 楼 songgz 2012-06-11  

你是一个会思考的人，前途无量，代码审查显然不能以风格和规则为主，那只是九牛一毛的事，或者说自然而然的事，高手心中是没有规则的。规则是小毛贼为了照猫画虎，硬制造的，简直就东妃效颦。代码审查当然以逻辑，结构，性能为主，甚至于架构和设计。何必据于形式呢，什么可读性啊，都是胡扯，只有要自身能力高，一切皆可读，不然好的设计，你还读不明白呢，就说不可读吗？

1 楼 Leon.Wood 2012-06-09  

写出让计算机读懂的代码很容易,写出让人读懂的只有高手才能做到.
华为连tab都不许用,缩进必须4个空格.存在即合理.