标签(空格分隔): 翻译 技术 C/C++ 作者:Andrey Karpov 翻译者:顾笑群 - Rafael Gu 最后更新:2017年02月05日
我们这里要说的和之前说到的“不要试图把尽量多的操作符放到一行代码里”有一些类似,但这次我想聚焦于另外一个方面。有时候我们程序员充满斗志,好像在和某人比赛一般,然后就会写一些尽量短(译者注:而且故意很难读看上去很高深的代码)。
我这里并非说的是那些复杂的模板。因为很难找到明显的界限去区分这些模板那里不好或那里好,所以这应该是一个另外一个话题的范畴。现在我们这里将要讨论的只是一些相关C/C++程序员的简单情况——经常的,这些程序员会写很复杂的代码,然后想表达“看我能这么玩,牛逼吧”。
下面这段代码来自KDE4。PVS-Studio诊断的错误描述为:V593 Consider reviewing the exPRession of the ‘A = B == C’ kind. The expression is calculated as following: ‘A = (B == C)’(译者注:大意是建议review代码,因为编译器会把其中类似A = B == C的语句理解为A = (B == C))。
void LDAPProtocol::del( const KUrl &_url, bool ){ .... if ( (id = mOp.del( usrc.dn() ) == -1) ) { LDAPErr(); return; } ret = mOp.waitForResult( id, -1 ); ....}解释 在看过上面的代码后,我一直有类似的疑问:把代码写成那样到底是图样?难道节省了一行空间吗?或者是作者想展示自己有能力把多个动作里和在一个表达式里?
但结果是我们得到了一个典型的错误模式——即使用了类似if (A = Foo() == Error)的表达式。
比较操作符的优先级高于赋值操作符,这就是为什么“mOp.del(usrc.dn()) == -1”表达式会先被执行的原因,然后只有true(1)或者false(0)被赋予了id变量。
如果mOp.del()返回了‘-1’,这个函数就会终止;否则程序就会继续执行,而且id变量也被赋予了一个不正确的值,这个判断也会始终等于0。
正确的代码 我想强调一下:增加额外的括号并不是这个问题的解决方法。是的,错误被消除了,但这是个错误的方法。
如果你靠近看,你会发现代码中已经有了额外的括号。很难判断为何要多加这个括号,也许是作者想去除编译器警告,也许是对操作符的优先级不确认,或者是想修复前面提到的问题,无论如何都失败了,所以增加额外的括号并没有提供帮助(译者注:其实是把括号位置放错了)。
但更深层次的问题在于,如果能让代码不太复杂,那就不要太复杂。所以正确的代码如下:
id = mOp.del(usrc.dn());if ( id == -1 ) {建议 不要因为懒惰而不想多写一行代码:毕竟复杂的表达式难以阅读。先做赋值,然后再比较。这样,你会让后来接手这些代码的程序员更容易一些,而且还减少了犯错误的几率。
所以,我的结论是——不要装逼。
这次这篇文章有些琐碎,但我希望能帮助你。写出干净整齐的代码始终是更好的方式,而不是那种“看我多牛逼”的方式。
新闻热点
疑难解答