近期code review几处小问题集锦
1 線程池使用不當
? ? ?我們的調度系統需要將一堆會員分配給相應的人員來處理，流程如以下偽代碼所示：
| 1 2 3 4 5 6 7 8 9 10 11 12 | public?void?dispatch() { ????????while?(true) { ????????????List<Member> memberList = getUnassignedMemberList();?//獲取所有未分配的會員 ????????????for(Member each : memberList) { ????????????????singleDispatch(each);??//為每一個會員分配相應的人員處理 ????????????} ????????????try?{ ????????????????Thread.sleep(1000);?//休眠1秒后繼續分配 ????????????}?catch?(InterruptedException e) { ????????????} ????????} ????} |
? ? 為了提高分配的速度，我們打算采用多線程的分配方式。一開始使用的是newCachedThreadPool。
| 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 | private?static??ExecutorService executor = Executors.newCachedThreadPool(); ????public?void?dispatch() { ????????while?(true) { ????????????List<Member> memberList = getUnassignedMemberList();?//獲取所有未分配的會員 ????????????for(final?Member each : memberList) { ????????????????executor.submit(new?Runnable() { ????????????????????@Override ????????????????????public?void?run() { ????????????????????????singleDispatch(each);??//為每一個會員分配相應的人員 ????????????????????} ????????????????}); ????????????} ????????????try?{ ????????????????Thread.sleep(1000);?//休眠1秒后繼續分配 ????????????}?catch?(InterruptedException e) { ????????????} ????????} ????} |
? ? 在壓測時發現，load飆升得很高，通過抓棧發現開啟了很多線程。原因是：newCachedThreadPool最大線程數為整型的最大值，每提交一個任務，如果沒有線程處理，那就產生一個新的線程。當我們for循環提交任務時，開辟了上百個線程，應用程序馬上崩潰。
? ? 既然發現了原因，我們馬上將調整線程池為newFixedThreadPool，這里我們可以設置最大線程數為4，隊列長度為整型的最大值。
| 1 | private?static?ExecutorService executor = Executors.newFixedThreadPool(4); |
? ? 但是壓測又發現新問題，線程池里的隊列長度不斷增長，而且分配不斷有異常拋出（異常信息為會員已經被分配過）。
? ? 原因是：當未分配會員較多時，可能需要5秒才能分配完，然而executor.submit是異步操作，當休眠1秒鐘后，馬上又進入下一個循環，隊列里又將插入重復的會員，這會導致隊列長度不斷增長，此外，會導致1個會員被分配后，又繼續被分配，導致異常產生。
? ? 解決方法：使用invokeAll這一同步語句，意思是只有當提交任務都被執行完后，才執行后續語句。
| 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 | public?void?dispatch() { ????????while?(true) { ????????????List<Callable<String>> tasks =?new?ArrayList<Callable<String>>(); ????????????List<Member> memberList = getUnassignedMemberList();?//獲取所有未分配的會員 ????????????for?(final?Member each : memberList) { ????????????????tasks.add(new?Callable<String>() { ????????????????????@Override ????????????????????public?String call() { ????????????????????????singleDispatch(each); ????????????????????????return?"ok"; ????????????????????} ????????????????}); ????????????} ????????????try?{ ????????????????executor.invokeAll(tasks,?480, TimeUnit.SECONDS);?//如果8分鐘還未執行完，則超時重新再來（魯棒性保證） ????????????}catch?(Exception e) { ????????????} ????????????try?{ ????????????????Thread.sleep(1000);?//休眠1秒后繼續分配 ????????????}?catch?(InterruptedException e) { ????????????} ????????} ????} |
2 NPE（java.lang.NullPointerException）
2.1 情形1
| 1 2 3 | if(case.getType() == Case.TYPE_SELF) { ????????????... } |
? ? 這段代碼拋出NPE時，直覺認為case為null導致的，后來打日志發現case并不為null，而case.getType()返回值類型為Integer，為null。
? ? 最后發現Case.TYPE_SELF返回的是int類型，而case.getType()是個null，null與int兩者一比較就報NPE。
? ? 這個問題的詭異之處我們直覺上認為Case.TYPE_SELF是個Integer，所以導致排查問題花費了些時間，因此一個建議這種常量如Case.TYPE_SELF都改為Integer，然后對象間比較的時候使用equals，并增加null判斷，就可以避免出現問題。
| 1 | if(null?!=?case.getType() &&?case.getType().equals(Case.TYPE_SELF)) |
2.2 情形2
| 1 | Integer leftNum = (null?!= leftNumMap && !leftNumMap.isEmpty()) ? leftNumMap.get(stat.getDepartmentId()) :?0; |
? ? 這個也拋NPE，我們排查了半個多小時，百思不得其解，后來查看class文件發現了原因。
? ? 假設我們代碼是：
| 1 2 | Map<String, Integer> leftNumMap =?new?HashMap<String , Integer>(); ????????Integer leftNum = (null?!= leftNumMap && !leftNumMap.isEmpty()) ? leftNumMap.get("test") :?0; |
? ? 反編譯后的代碼如下：
| 1 2 | HashMap leftNumMap =?new?HashMap(); ????????Integer leftNum = Integer.valueOf(null?!= leftNumMap && !leftNumMap.isEmpty()?((Integer)leftNumMap.get("test")).intValue():0); |
? ? ?為什么會這樣呢？《你真的會用 Java 中的三目運算符嗎？》一文做了說明。三目運算符的語法規范是這樣寫的：If one of the second and third operands is of primitive type T, and the type of the other is the result of applying boxing conversion (§5.1.7) to T, then the type of the conditional expression is T.
? ? ?簡單的來說就是：當第二，第三位操作數一個為基本類型一個為對象時，其中的對象就會拆箱為基本類型進行操作。
? ? ?所以，結果就是：由于使用了三目運算符，并且第二、第三位操作數一個是基本類型一個是對象。所以對對象進行拆箱操作，由于該對象為null，所以在拆箱過程中調用null.intValue()的時候就報了NPE。
? ? ?解決方法很簡單：1）要么不用三目運算符，直接使用if else，簡單可靠；2）或者將三目運算符操作數都改為對象。
| 1 | Integer leftNum = (null?!= leftNumMap && !leftNumMap.isEmpty()) ? leftNumMap.get(stat.getDepartmentId()) : Integer.valueOf(0); |
3 Map<K, List<V>>用錯
? ? ?我們代碼中經常出現以下這種數據結構，比如key是類目，value是個List，list存儲著這個類目下各個子類目的統計數據等。
| 1 | Map<String,List<MyClass>> myClassListMap =?new?HashMap<String,List<MyClass>>() |
? ? 我們插入該數據結構的時候往往得這樣寫：
| 1 2 3 4 5 6 7 8 | void?putMyObject(String key, Object value) { ????List<Object> myClassList = myClassListMap.get(key); ????if(myClassList ==?null) { ????????myClassList =?new?ArrayList<object>(); ????????myClassListMap.put(key,myClassList); ????} ????myClassList.add(value); } |
? ? 當我們希望檢查List中的對象是否存在，或刪除一個對象，那要遍歷整個數據結構，需要更多的代碼。
? ? 這些代碼不僅給可讀性帶來障礙，更提高了大家出錯的概率，比如某一次我們在復雜的業務邏輯中實現putMyObject時，漏寫了句:
| 1 | myClassListMap.put(key,myClassList); |
? ? 結果導致花費了大量時間才發現這個問題。
? ? 當然，細致的測試以及code review能避免出現類似問題，但是有沒有一種方法既能從技術上來幫助我們避免此類問題，又能節約代碼提高代碼可讀性？
? ? 推薦大家使用google guava包，里面提供了MultiMap數據結構，使用方式如下，非常簡潔明了，也不會有機會讓我們出錯。
| 1 2 3 4 5 6 7 8 9 | Multimap<String, String> myMultimap = ArrayListMultimap.create(); ????????myMultimap.put("女裝",?"內衣"); ????????myMultimap.put("女裝",?"羽絨服"); ????????myMultimap.put("女裝",?"風衣"); ????????myMultimap.put("男裝",?"皮夾克"); ????????// 獲取key "女裝"對應的list ????????Collection<String> womenDressList = myMultimap.get("女裝"); ????????// 刪除key "女裝"對應List中的"羽絨服" ????????myMultimap.remove("女裝",?"羽絨服"); |
轉載請標明源地址：http://www.cnblogs.com/LBSer
總結
以上是生活随笔為你收集整理的近期code review几处小问题集锦的全部內容，希望文章能夠幫你解決所遇到的問題。
- 上一篇： 常用Apache Commons工具类备
- 下一篇： 读阿里许令波老师晋升评审有感