標簽(空格分隔): 翻譯 技術 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 ) {建議 不要因為懶惰而不想多寫一行代碼:畢竟復雜的表達式難以閱讀。先做賦值,然后再比較。這樣,你會讓后來接手這些代碼的程序員更容易一些,而且還減少了犯錯誤的幾率。
所以,我的結論是——不要裝逼。
這次這篇文章有些瑣碎,但我希望能幫助你。寫出干凈整齊的代碼始終是更好的方式,而不是那種“看我多牛逼”的方式。
新聞熱點
疑難解答