「每日分享」CodeReview 常見代碼問題(下)

點擊上方"java全棧技術"關注,每天學習一個java知識點

「每日分享」CodeReview 常見代碼問題(下)

可維護性問題

可維護性問題是“在當前業務變更的範圍內通常不會導致BUG、故障,卻會在日後埋下地雷,引發BUG、故障、維護成本大幅增加”的類別。

硬編碼

硬編碼主要有三種情況: a. “魔數”; b. 寫死的配置; c. 臨時加的邏輯和文案。

“魔數”與重複代碼類似,當前或許不會引發問題,時間一長,為了弄清楚其代表的含義,增加很多溝通維護成本,且分散在各處很容易導致修改的時候遺漏不一致。務必清清除。方法也比較簡單:定義含義明顯的枚舉或常量,代表這個魔數在代碼中發言。

“寫死的配置”不會影響業務功能, 不過在環境變更或系統調優的時候,就顯得很不方便了。 方法: 儘量將配置抽離出來做成配置項放到配置文件裡。

“臨時加的邏輯和文案”也是一種破壞系統可維護性的做法。方法: 抽離出來放在單獨的函數或方法裡,並特別加以註釋。

重複代碼

重複代碼在當前可能不會造成 BUG,但上線後,需要維護多處的事實一致性;時間一長,後續修改的時候就特別容易遺漏或處理不一致導致 BUG;重複代碼是公認的“代碼壞味”,必當盡力清除。方法: 抽離通用的部分,定製差異。重複代碼還有一種情況出現,即創造新函數時,先看看是否既有方法已經實現過。

通用邏輯與定製業務邏輯耦合

這大概是每個媛猿們在開發生涯中遇到的最噁心的事情之一了。通用邏輯與具體的各種業務邏輯混雜交錯,想插根針都難。遇到這種情況,只能先祈福,然後抽離一個新的函數,嚴格判斷相應條件滿足後去調用它。

如果是新創建邏輯,可以使用函數式編程或基於接口的編程,將通用處理流程抽離出來,而將具體業務邏輯以回調函數的形式傳入處理。

不要讓不同的業務共用相同的函數,然後在函數里一堆 if-else plus switch , 而是每個業務都有各自的函數, 並可複用相同的通用邏輯和流程處理; 或者各個業務可以覆寫同樣命名的函數。

複用,而非混雜。

直接在原方法里加邏輯

有業務改動時,猿媛們圖方便傾向於直接在原方法里加判斷和邏輯。這樣做是很不好的習慣。一方面,增加了原方法的長度,破壞了其可維護性;另一方面,有可能對原方法的既有邏輯造成破壞。 可靠的方式是: 新增一個函數,然後在原方法中調用並說明原因。

多業務耦合

在業務邊界未仔細劃分清晰的情況下出現,一個業務過多深入和摻雜另一個非相關業務的實現細節。在項目和系統設計之初,特別要注意先劃分業務邊界,定義好接口設計和服務依賴關係,再著手開發;否則,延遲到後期做這些工作,很可能會導致重複的工作量,含糊複雜的交互、增加後期系統維護和問題排查的許多成本。磨刀不誤砍柴工。劃分清晰的業務、服務、接口邊界就屬於磨刀的功夫。

代碼層次不合理

代碼改動邏輯是正確的,然而代碼的放置位置不符合當前架構設計約定,導致後續維護成本增加。

代碼層次不合理可能導致重複代碼。比如獲取操作人和操作記錄,如果寫在類 XController 裡, 那麼類 YController 就面臨尷尬局面: 如果寫在 YController , 就會導致重複代碼; 如果跨層去調用 XController 方法,又是非常不推薦的做法。因此, 獲取操作人和操作記錄,最好寫在 Service 層, Controller 層只負責參數傳入、檢測和結果轉譯、返回。

不用多餘的代碼

工程中常常會有一些不用的代碼。或者是一些暫時未用到的Util工具或庫函數,或者是由於業務變更導致已經廢棄不用的代碼,或者是由於一時寫出後來又重寫的代碼。儘量清除掉不用多餘的代碼,對系統可維護性是一種很好的改善,同時也有利於CodeReview。

使用全局變量

使用全局變量並沒有“錯”,錯的是,一旦出現問題,排查和調試問題起來,真的會讓人“一夜之間白了頭”,耗費數個小時是輕微懲罰。此外,全局變量還能“順手牽羊”地破壞函數的通用性,導致可維護性變差。務必消除全局變量的使用。當然,全局常量是可以的。

缺乏必要的註釋

對重要和關鍵點的代碼缺乏必要的註釋,使用到的重要算法缺乏必要的引用出處,對特別的處理缺乏必要的說明。

原則上, 每個方法至少要用一個簡短的單行註釋, 適宜地描述了方法的用途、業務邏輯、作者及日期。對於特殊甚至奇葩的需求的特別實現,要加一些註釋。 這樣後續維護時有個基礎。

更難發現的錯誤

更難發現的錯誤是指“複雜併發場景下的有一定技術難度的、需要豐富開發與設計經驗才能看出來的錯誤”。

併發

併發的問題更難檢測、復現和調試。

常見的問題有:

a. 在可能由多線程併發訪問的對象中含有共享變量卻沒有同步保護;

b. 在代碼中手動創建缺乏控制的線程或線程池;

c. 併發訪問數據庫時沒有做任何同步措施;

d. 多個線程對同一對象的互斥操作沒有同步保護。

對於 a, 在大部分Java應用中,通常由Spring框架來控制和創建請求和服務實例,因此,保證“Controller, Service 類中的實例變量只允許 Service, DAO 的單例,不允許業務變量實例”基本確保沒有併發不正確更新的問題;不過,包含緩存策略的對象要特別注意多線程併發訪問的問題,出於性能考量, 儘量只對共享實例部分加鎖。

對於 b, 禁止在應用中手動創建線程或線程池,失控的線程池很容易導致應用崩潰(有線上應用崩潰的教訓)。

對於 c, 併發訪問數據庫時,要特別注意時序和狀態同步。如果時序控制不對,會導致狀態同步和更新出錯。

對於 d, 對同一對象的互斥操作需要加分佈式鎖同步。

使用線程池、併發庫、併發類、同步工具而不是線程對象、併發原語。在複雜併發場景下,還需注意多個同步對象上的鎖是否按合適的順序獲得和釋放以避免死鎖,相應的錯誤處理代碼是否合理。

資源洩露

  • 打開文件卻沒有關閉;
  • 連接池的連接未回收;
  • 重複創建的腳本引用沒有置空,無法被回收;
  • 已使用完的集合元素始終被引用,無法被回收;

事務

事務方面常出現的問題是:多個緊密關聯的業務操作和 SQL 語句沒有事務保證。 在資金業務操作或數據強一致性要求的業務操作中,要注意使用事務,保證數據更新的一致性和完整性。

SQL問題

SQL的正確性通常可以通過 DAO 測試來保證。 SQL問題主要是指潛在的性能問題和安全問題。

要避免SQL性能問題, 在表設計的時候就要做好索引工作。在表數據量非常大的情況下,SQL語句編寫要非常小心。查詢SQL需要添加必要索引,添加合適的查詢條件和查詢順序,加快查詢效率, 避免慢查; 儘量避免使用 Join, 子查詢;避免SQL注入。

尤其避免在 update 語句中使用 where-if ! 很容易導致全表更新和嚴重的數據丟失,造成嚴重的線上故障 !!!

SQL優秀書籍推薦: SQL語言藝術

安全問題

安全問題一向是互聯網產品研發中極容易被忽視、而在爆發後又極引發熱議的議題。安全和隱私是用戶的心理紅線之一。應用、數據、資金的安全性應當僅次於產品功能的準確性和使用體驗。

比如:緩衝區溢出; 惡意代碼注入;權限賦予不當; 應用目錄洩露等。

安全問題的CodeReview可參見檢查點清單:信息安全 。主要是如下措施: a. 嚴格檢查和屏蔽非法輸入; b. 對含敏感信息的請求加密通信; c. 業務處理後消除任何敏感私密信息的任何痕跡; d. 結果返回前在反序列化中清除敏感私密信息; e. 敏感私密信息在數據存儲設備中應當加密存儲; f. 應用有嚴格的角色、權限、操作、數據訪問分級和控制; g. 切忌暴露服務器的重要的安全性信息,防止服務器被攻擊影響正常服務運行。

設計問題

設計問題通常體現在: a. 是否有潛在的性能問題; b. 是否有安全問題; c. 業務變化時是否容易擴展; d. 是否有遺漏的點; e. 持續高負荷壓力下是否會崩潰。

較輕微的問題

較輕微問題是指“沒有技術難度、通過良好習慣即可避免的問題”。

較輕微問題一般不會造成負面影響的BUG或故障,不過建立一些好的習慣,主動使用代碼檢測工具,消除這些較輕微錯誤,也是一種修行。

命名不貼切

命名不貼切不會影響功能實現,卻會誤導理解或增加理解難度。

方法:先查查字典,找個通俗易懂而且比較貼近的名字。可以參考 jdk 的命名、通用詞彙和行業詞彙; 作用域小的採用短命名,作用域大的採用長命名。取名字是一種重要技能,—— 多少父母為此愁灰了頭!

聲明時未初始化通常情況下都不會是問題,因為後面會進行賦值。不過,如果賦值的過程中出現異常,那麼可能會返回空值,從而導致空值異常。通常,變量聲明時賦予默認初始值是個好習慣。

風格與整體有不一致

工程通常求穩,一致性能更好地維護。在工程項目中,最好能夠遵循工程約定的風格,在個人項目中可以凸顯個性風格。Java編程一般要遵循《Java編程規範》,有追求的程序猿媛還會追求更高層次的,比如《Google Java 規範》等。

類型轉換錯誤

編程語言的類型系統是非常重要的。如何在不同類型之間可靠地互轉,尤其是在父子類型之間相互賦值,也是一個微技能。濫用類型轉換,也會導致BUG 。

Java 中容易出現的錯誤是:a. 字符串轉數值,字符串含有非數字部分;b. JSON字符串轉對象,某個字段含有不兼容的值類型導致解析出錯;c. 子類型轉不兼容的父類型,滋生運行時異常 ClassCastException;d. 相同特質的類型不兼容。比如 Long 與 Integer 都是數值型,卻不能互轉。

類型轉換中最容易出BUG的地方是非布爾類型取反。受C語言的影響,很多高級語言支持各種數據類型轉布爾類型,比如 PHP 字符串、數組、數字等都可以轉布爾類型,相應的就喜歡寫 if (!notBoolVar) 這種表達式, 容易隱藏看不出的BUG甚至錯誤。

否定式風格

變量含義、表達式語句傾向於使用否定式風格,可能不知不覺耗費大量腦細胞,因為每次理解的時候都要繞個彎子。 比如 isNoExpress 是否無需物流, 就有點繞。 為什麼呢? 無需物流是針對快遞發貨的, 如果快遞發貨佔發貨的90%, 無需物流只佔10%,那麼, isNoExpress = false 幾乎總為真。 涉及到判斷的時候,可能不得不寫 if (!isNoExpress) , 雙重否定足夠弄暈你。

容器遍歷的結構變更

絕大多數語言都承襲了 C 語言的 for(int i=0;i

API參數傳遞錯誤

如果API參數有多個,而且相鄰參數的類型相同,那麼要特別留意是否參數順序是正確的,而不會張冠李戴。

當然,在設計API參數的時候,就可以仔細用更精準類型進行區分,並將相同類型的參數錯開。比如 calc(int accountNo, int pay, int timestamp) , 就容易傳錯,比較可靠的是 calc(int accountNo, Currency pay, Timestamp now) ,這樣是不可能將參數傳遞錯誤的。

單行調用括號過多

為了簡便,常常會寫出 wapper(calc(now, String.format("%s\n", new BufferedFileReader(filename, "UTF-8").readLines() ))) 的語句 , 嗯,你得好好瞧瞧和算算右邊的括號數量是否正確了。更糟糕的時候,結合API參數傳遞錯誤,IDE 可能沒有報錯, 而你很可能沒有意識到自己的參數傳遞錯誤了。 可靠的方式是, 拆出一部分變量,並將調用之間的括號用空格隔開,顯示出層次感。

「每日分享」CodeReview 常見代碼問題(下)

修改方法簽名

對某個方法有業務改動時,程序猿媛們傾向直接修改原方法的簽名。這時,要特別注意:a. 不要修改原方法的參數順序; b. 在最後面增加可選參數。 從另一個角度來看,複雜的業務方法應當分兩層: 最外層負責調度,方法參數具有包容性,裡面包含的字段比較多 ; 內層方法負責特定業務邏輯的實現,方法參數少而精。

修改原方法簽名本身就是容易產生問題的習慣, 篡改原方法的參數順序更是大忌。 最好的方法是新建一個方法去複用原方法, 然後調用新的方法。代碼變更始終銘記“開閉”原則。

打印日誌太多

打印過多的日誌並不好。一方面遮掩真正需要的信息,導致排查耗費時間, 另一方面造成服務器空間浪費、影響性能。生產環境日誌一般只開放 INFO及以上級別的日誌; Debug 日誌只在調試或排錯的時候使用,生產環境可以禁止debug日誌。

多級數據結構

使用多級數據結構時,要確定父級數據一定有值,或者進行檢測。比如 $order['baole']['ump']['money'],必須確保 $order['baole'], $order['baole']['money'] 一定有值或做非空檢測。

作用域過大

由於C語言的影響,猿媛們會在開頭就定義好一些變量或要返回的對象,在很靠後的地方才使用到。不必要的過大的作用域對變量和對象的變化產生不可測的影響,並增大理解的成本。可靠的方法是,僅當在使用時才定義,並儘快返回結果。

另一種情況是,暴露的訪問域過大,比如 public 字段。 儘可能地縮小可訪問的範圍,可以增大變更和重構的空間; 減少可變性,則可以自然地獲得併發安全性,降低CodeReview的理解成本。

比如,不可變的類和字段定義成 final , 最小化包,類,接口,方法和域的可訪問性,默認為 private , 若需要繼承,可定義為 protected , 僅當需要作為 API 服務暴露出去時,使用 public.

分支與循環

條件與循環偶爾也會導致錯誤, 不過通常錯誤可以在發佈前解決掉。

對於 if-else 嵌套條件, 需要仔細檢查是否符合業務邏輯; 如果嵌套太深,是否可以使用另一種方式“解結” ; 對於 switch 語句, 大多數語言的 case 有 fall through 問題, 要注意加上 break ; 最好加上 default 的處理。

對於 for 循環, 編寫合理的結束條件避免死循環; 對於循環變量的控制, 避免出現 -1或 +1 錯誤, 消除越界錯誤; for 循環也要特別注意對空值和空容器的處理,避免拋出空值異常。可以通過單測來確保 for 循環的準確性。

殘留的無用代碼

殘留的無用代碼,會成為系統的垃圾,增加系統的理解和維護成本。需要及時清理掉。

代碼與文檔不一致

文檔是理解代碼的第一扇窗口。優秀的文檔,可以極大地降低理解代碼的成本。但是大多數開發者還並不習慣編寫友好的文檔。常常出現無文檔、失效文檔、誤導性文檔等,影響人們的理解和判斷。

使用冷僻用法或奇淫巧技

使用冷僻用法或奇淫巧技會增大系統的理解成本,徒然消耗人的腦細胞。思路可借鑑,但不宜用於生產環境中。樸實最宜。

作者:琴水玉 原文:http://www.cnblogs.com/lovesqcc/p/9271781.html


分享到:


相關文章: