代碼細節重構：請對我的代碼指手劃腳（四）

這是上周在代碼審閱會議上討論到的一段代碼，這段代碼的作用是根據指定記錄數量和頁面大小來計算最大分頁數量的。

目標代碼

 1 /// <summary> 2 /// 根據指定記錄數量和頁面大小返回分頁頁數。 3 /// </summary> 4 /// <param name="totalRecords">記錄總數。</param> 5 /// <param name="pageSize">頁面大小。</param> 6 /// <returns>返回分頁頁數。</returns> 7 public static long ComputePages(int totalRecords, int pageSize) 8 { 9     var total = Convert.ToDouble(totalRecords);10     var size = Convert.ToDouble(pageSize);11 12     var temp = total / size;13 14     return (long)Math.Ceiling(temp);15 }

代碼問題

在代碼審閱會議上起初并沒有留意到這段代碼，但隨著翻閱了多個代碼頁之后便發現“ComputePages”這個方法被多次重新定義，且功能完全一樣。應該是開發人員復制粘貼過來的，原本也是想針對復制粘貼來講講這段代碼的，但后來發現了更多的問題：

1. 代碼重用問題。通過在整個解決方案搜索，發現這個方法被原封不動的復制粘貼了36次，總計在37個類當中出現了37次，而且這些類全部都是靜態類。這里的問題就是對于重復出現的代碼邏輯，而且是業務邏輯簡單且又完全一樣的代碼，沒能實現代碼重用，這會給后續的代碼維護帶來諸多不便；
2. 類型安全問題。這段代碼的操作對象都是數字，數字屬于值類型，類型安全問題不會顯得特別突出。但數字本身卻會有一些其他的隱含問題，比如數據類型轉換、分母不能為零等；
3. 分母不能為零。很顯然，這段代碼里面包含了除法，而對于除法運算沒有做分母不能為零的邏輯處理。對于新手程序員來說這是很容易被忽略的問題，但對于有一定經驗的研發人員來講，這一點勢必已經刻骨銘心了；
4. 代碼性能問題。我們知道在托管代碼中進行各種類型轉換都會帶來不小的性能損失，而如上代碼便存在著一些可優化的部分；
5. 業務邏輯問題。認真觀察和分析如上代碼，你會發現該方法的輸入參數在特例條件下會輸出不符合實際期望的結果。比如輸出結果可能是小于0的負數；

重構建議

找到了問題所在，那么就會有一定的解決方案：

• 對于代碼重用，可以在某個業務邏輯相關的類中集中定義這個方法，而不是分散到不同的類中。不過，本文不會具體討論這個問題；
• 對于類型安全、類型轉換，具體到數字的時候，我們應該適當的考慮一下通過數學的方法來解決這些問題，而不是死板的套取程序語言規范；
• 對于分母不能為零這一類定理性的問題，我們在編碼過程中一定要小心在意！千萬不能抱著僥幸心理；
• 對于業務邏輯上的問題，我們在編碼之前一定要認真思考、琢磨，弄清楚要解決的問題及可能產生的后果；

重構結果

為了使得大家更容易理解，我將代碼重構的一些具體思路和想法以備注的方式貼在代碼里面了，大家認真觀察，有問題可以提出。

 1 /// <summary> 2 /// 根據指定記錄數量和頁面大小返回分頁頁數。 3 /// </summary> 4 /// <param name="totalRecords">記錄總數。</param> 5 /// <param name="pageSize">頁面大小。</param> 6 /// <returns>返回分頁頁數。</returns> 7 /*  8     * 1、方法返回值從long修改為int。原因如下： 9     *    ·輸入參數皆為int，且運算關系為除法，所以輸出結果必定在int范圍之內。不使用unit是因為那會給整個解決方案的其他部分代碼帶來很多的編碼痛苦；10     *    ·小小的內存性能改進；11     */12 public static int ComputePages(int totalRecords, int pageSize)13 {14     /*15         * 2、對記錄總數的類型安全判斷和業務邏輯處理：16         *    ·記錄總數肯定是大于等于零的數；17         *    ·記錄總數為零的時候，頁數肯定也是零，此時后續的代碼邏輯沒有意義，可以直接返回0；18         */19     if (totalRecords < 1) return 0;20     /*21         * 3、對頁面大小的類型安全判斷和業務邏輯處理：22         *    ·頁面大小不能為0，因為分母不能為0；23         *    ·頁面大小不能為負數，因為這與事實不符；24         *    ·當頁面大小為1的時候，頁數其實就是記錄總數，直接返回totalRecords，這同時也避免了后續的代碼無意義的執行，減少了性能損耗；25         */26     if (pageSize < 1) return totalRecords;27 28     /*29         * 4、對原方法業務邏輯的重構、類型轉換方式的優化及性能優化：30         *    ·通過任何數字乘以（或除以）浮點數其結果仍然為浮點數的方式完成自動的、隱式的類型轉換，這不但使得代碼更加簡潔，且優化了性能；31         *    ·代碼簡單、可讀性強，更加容易理解業務邏輯；32         */33     return (int)Math.Ceiling(totalRecords*1.0/pageSize);34 }

結語

俗話說細節決定成敗。作為程序員，從代碼的細節我們就能看得出一個人編程水平的高低和開發經驗的多寡，這也從一定意義上決定了你的成與??！

之前我發表過很多文章，都是從人文角度來引導新手的，今天這篇文章，也正好說明了人文因素對程序員素質的影響！

要做事，請首先做好人，對自己的代碼負責就是對自己負責，對自己負責就是對團隊負責，對團隊負責就是對企業利益負責，因為我們是同一個團隊，擁有同樣的利益！