近期code review幾處小問題集錦

1 線程池使用不當

     咱們的調度系統須要將一堆會員分配給相應的人員來處理，流程如如下僞代碼所示：

public void dispatch() {
        while (true) {
            List<Member> memberList = getUnassignedMemberList(); //獲取全部未分配的會員
            for(Member each : memberList) { 
                singleDispatch(each);  //爲每個會員分配相應的人員處理
            }
            try {
                Thread.sleep(1000); //休眠1秒後繼續分配
            } catch (InterruptedException e) {
            }
        }
    }

    爲了提升分配的速度，咱們打算採用多線程的分配方式。一開始使用的是newCachedThreadPool。

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，隊列長度爲整型的最大值。

private static ExecutorService executor = Executors.newFixedThreadPool(4);

    可是壓測又發現新問題，線程池裏的隊列長度不斷增加，並且分配不斷有異常拋出（異常信息爲會員已經被分配過）。
    緣由是：當未分配會員較多時，可能須要5秒才能分配完，然而executor.submit是異步操做，當休眠1秒鐘後，立刻又進入下一個循環，隊列裏又將插入重複的會員，這會致使隊列長度不斷增加，此外，會致使1個會員被分配後，又繼續被分配，致使異常產生。
    解決方法：使用invokeAll這一同步語句，意思是隻有當提交任務都被執行完後，才執行後續語句。

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

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判斷，就能夠避免出現問題。

if(null != case.getType() && case.getType().equals(Case.TYPE_SELF))

2.2 情形2

Integer leftNum = (null != leftNumMap && !leftNumMap.isEmpty()) ? leftNumMap.get(stat.getDepartmentId()) : 0;

    這個也拋NPE，咱們排查了半個多小時，百思不得其解，後來查看class文件發現了緣由。
    假設咱們代碼是：

Map<String, Integer> leftNumMap = new HashMap<String , Integer>();
        Integer leftNum = (null != leftNumMap && !leftNumMap.isEmpty()) ? leftNumMap.get("test") : 0;

    反編譯後的代碼以下：

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.

     簡單的來講就是：當第二，第三位操做數一個爲基本類型一個爲對象時，其中的對象就會拆箱爲基本類型進行操做。
     因此，結果就是：因爲使用了三目運算符，而且第2、第三位操做數一個是基本類型一個是對象。因此對對象進行拆箱操做，因爲該對象爲null，因此在拆箱過程當中調用null.intValue()的時候就報了NPE。
     解決方法很簡單：1）要麼不用三目運算符，直接使用if else，簡單可靠；2）或者將三目運算符操做數都改成對象。

Integer leftNum = (null != leftNumMap && !leftNumMap.isEmpty()) ? leftNumMap.get(stat.getDepartmentId()) : Integer.valueOf(0);

3 Map<K, List<V>>用錯

     咱們代碼中常常出現如下這種數據結構，好比key是類目，value是個List，list存儲着這個類目下各個子類目的統計數據等。

Map<String,List<MyClass>> myClassListMap = new HashMap<String,List<MyClass>>()

    咱們插入該數據結構的時候每每得這樣寫：

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時，漏寫了句:

myClassListMap.put(key,myClassList);

    結果致使花費了大量時間才發現這個問題。
    固然，細緻的測試以及code review能避免出現相似問題，可是有沒有一種方法既能從技術上來幫助咱們避免此類問題，又能節約代碼提升代碼可讀性？
    推薦你們使用google guava包，裏面提供了MultiMap數據結構，使用方式以下，很是簡潔明瞭，也不會有機會讓咱們出錯。

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("女裝", "羽絨服");