1、申 请 I D:lumou
2、个人邮箱:987122239@qq.com
3、原创技术文章:看雪精华贴地址:http://bbs.pediy.com/thread-215956.htm (因为是翻译的文章,不知道能不能符合要求。还有文章太长,可否只贴第一章。)
1.别做编译器的活
来看一段选自 MySQL 项目代码块,这段代码包含有一个错误,在 PVS-Studio 代码分析器中表现为:V525 代码包含一系列相同的代码块。检查在 680,682,684,684,689,691,693,695 行的“0”,“1”,“2”,“3”,“4”,“1”,“6”项
[C++] 纯文本查看 复制代码 static int rr_cmp(uchar *a,uchar *b)
{
if (a[0] != b[0])
return (int) a[0] - (int) b[0];
if (a[1] != b[1])
return (int) a[1] - (int) b[1];
if (a[2] != b[2])
return (int) a[2] - (int) b[2];
if (a[3] != b[3])
return (int) a[3] - (int) b[3];
if (a[4] != b[4])
return (int) a[4] - (int) b[4];
if (a[5] != b[5])
return (int) a[1] - (int) b[5]; <<<<====
if (a[6] != b[6])
return (int) a[6] - (int) b[6];
return (int) a[7] - (int) b[7];
}
解释
这是一个关于代码复制粘贴的典型错误。很显然,程序员复制代码“ if (a[1] != b[1]) return (int) a[1] - (int) b[1];”,然后开始改变下标,但是忘了将“1”改为“5”。结果导致在这个比较函数中会返回不正确的值。这个问题比较不容易注意到,而且很难被检测到,因为在我们使用 PVS-Studio 浏览 MySQL 之前所有的测死用力都没有检测出来。
正确代码
[C++] 纯文本查看 复制代码 if (a[5] != b[5]) return (int) a[5] - (int) b[5];
建议
尽管这段代码简洁、易读,但是开发人员还是比较容易忽视掉这个错误。在阅读类似这样的代码的时候你会比较难集中精力,因为你所看的都是相似的代码。这些相似的语句块经常会导致一个结果,就是程序员会想要尽可能的优化代码。他手动“展开循环”。我认为在这里,循环展开并不是一个好法子。首先,我会怀疑程序员能否真的从这里获得任何益处。现代的编译器都非常的智能,也非常的善于在可以提升代码能力的情况下自动展开循环。其次,这段代码块的 bug 是因为他想要优化代码。如果你把它写成一段比较简单的循环的话,犯错的几率应该会小一点。我会建议用下面的方式重写这个函数:
[C++] 纯文本查看 复制代码 static int rr_cmp(uchar *a,uchar *b)
{
for (size_t i = 0; i < 7; ++i)
{
if (a[i] != b[i])
return a[i] - b[i];
}
return a[7] - b[7];
}
优点:
- 函数更易于阅读和理解
- 犯错的几率比较小我敢肯定这个函数不会比原来那个版本运行慢。
所以,我的建议就是——写简单和易于理解的代码。具体来说,简单的代码就是正确的代码。不要尝试去做编译器的活——比方说,展开循环。如果展开会比较好的话,编译器会去做的。做这种手动优化的工作只有在一些比较重要的代码块那里才会有意义,而且这只有在profiler 已经将这些代码块判断为有问题(慢)的时候才成立。
登录截图
|