代碼檢視問題總結

案例一

首先來看一段代碼

        //根據Y01, Y03 匹配 公募基金一年、三年的數據
        for (ProfitRelative profitRelative : publicFund.getProfitRelativeList()) {
            if ("Y01".equals(profitRelative.getDuration())) {
                profits[0] = profitRelative.getProfitRate();
                if (profitRelative.getProfitRank() == null || profitRelative.getProfitRank() == 0 || profitRelative.getProfitRankTotal() == null || profitRelative.getProfitRankTotal() == 0){
                    exceedRank[0] = null;
                }else {
                    exceedRank[0] = 100 - (profitRelative.getProfitRank() * 100 / profitRelative.getProfitRankTotal());
                }
            } else if ("Y03".equals(profitRelative.getDuration())) {
                profits[1] = profitRelative.getProfitRate();
                if (profitRelative.getProfitRank() == null || profitRelative.getProfitRank() == 0 || profitRelative.getProfitRankTotal() == null || profitRelative.getProfitRankTotal() == 0){
                    exceedRank[1] = null;
                }else {
                    exceedRank[1] = 100 - (profitRelative.getProfitRank() * 100 / profitRelative.getProfitRankTotal());
                }
            }
        }

乍一看令人恐懼。下面我們一步一步地，對它進行重構。

重複是萬惡之源

重複 1：分支Y01 與 分支Y03，邏輯完全一致。此時我們執行第一個重構手段，將這段一樣的邏輯抽成一個方法。

private static double winPercent() {
    if (profitRelative.getProfitRank() == null || profitRelative.getProfitRank() == 0 || profitRelative.getProfitRankTotal() == null || profitRelative.getProfitRankTotal() == 0){
        return = null;
     }else {
           return 100 - (profitRelative.getProfitRank() * 100 / profitRelative.getProfitRankTotal());
     }
}

重複 2：判斷 int 是否為非空非0。此時我們執行同樣的重構手段

    private static boolean anyNullOrZero(Integer ...values) {
        for (Integer value : values){
            if (value == null || value == 0){
                return true;
            }
        }
        return false;
    }

    private static double winPercent(ProfitRelative profitRelative) {
        if ( anyNullOrZero(profitRelative.getProfitRank(), profitRelative.getProfitRankTotal()) ){
            return = null;
         } else {
            return 100 - (profitRelative.getProfitRank() * 100 / profitRelative.getProfitRankTotal());
         }
    }

翻轉判斷，減少分支

if 語句裏面儘可能不要用 !，而且儘可能突出核心邏輯。下面執行第二個重構手段。首先將

public class XXUtils {
    // 如果需要取反，直接加一個方法，比在判斷語句中使用`!`要好很多
    public static boolean noneNullOrZero(Integer ...values){
        return ! anyNullOrZero(values);
    }

    private static boolean anyNullOrZero(Integer ...values) {
        for (Integer value : values){
            if (value == null || value == 0){
                return true;
            }
        }
        return false;
    }
}

    private static double winPercent(ProfitRelative profitRelative) {
        // 翻轉，去除一個沒用的else分支
        if ( XXUtils.noneNullOrZero(profitRelative.getProfitRank(), profitRelative.getProfitRankTotal()) ){
            return 100 - (profitRelative.getProfitRank() * 100 / profitRelative.getProfitRankTotal());
         }
        return null;
    }

方法下沉，領域充血

getWinPercent() 應該下沉到類 ProfitRelative 裏面，作為一個成員方法。

這裏需要重點強調一下，很多同學喜歡把邏輯都往 service 裏面堆放，導致代碼貧血甚至失血。其實只要將邏輯放到領域對象裏面，就可以使邏輯清晰很多，讓代碼充血。

public class ProfitRelative {
    private Integer profitRank;
    private Integer profitRankTotal;

    public Integer getWinPercent(){
        if ( noneNullOrZero(profitRank, profitRankTotal )) {
            return 100 - (profitRank * 100 / profitRankTotal);
        }
        return null;
    }
}

最後，開始那坨代碼變成這樣：

        //根據Y01, Y03 匹配 公募基金一年、三年的數據
        for (ProfitRelative profitRelative : publicFund.getProfitRelativeList()) {
            if ("Y01".equals(profitRelative.getDuration())) {
                profits[0] = profitRelative.getProfitRate();
                exceedRank[0] = profitRelative.getWinPercent();

            } else if ("Y03".equals(profitRelative.getDuration())) {
                profits[1] = profitRelative.getProfitRate();
                exceedRank[1] = profitRelative.getWinPercent();
            }
        }

上面這坨代碼仍然有改進空間，可以繼續執行重構手法。這裏只是展示重構是如何一步一步實施的：小幅修改，測試，再修改。

案例二

看到這個案例，第一反應是難以理解，主要有幾點問題：

• java 8 stream 的濫用導致代碼難以理解
• List to Map，其實還是 O(n)，還不如直接遍歷好理解
• 重複
• null判斷

   public void changeRateByManager(){
       if (CollUtil.isNotEmpty(rateTypeList)){
           Map<String, List<RateType>> collect = rateTypeList.stream().collect(Collectors.groupingBy(RateType::getRateType));
           BigDecimal y031 = Optional.ofNullable(collect).map(ra -> ra.get("Y03")).map(y03 -> y03.get(0)).map(RateType::getRate).orElse(null);
           if (rate.compareTo(BigDecimal.ZERO)<0){
               if (null==y031 || y031.compareTo(BigDecimal.valueOf(-100))==0){
                   BigDecimal YBG = Optional.of(collect).map(ra -> ra.get("YBG")).map(y03 -> y03.get(0)).map(RateType::getRate).orElse(null);
                   if (YBG != null && YBG.compareTo(BigDecimal.valueOf(-100))!=0) {
                       rateDescription="成立以來收益率";
                       rate=YBG;
                   }
               }else{
                   rate=y031;
                   rateDescription="近3年收益率";
               }
           }
       }
   }

開始重構

從列表中取出對應的RateType，抽出一個方法getRateType()。需要注意的是，這裏不是返回null。這樣就避免了每次拿到RateType做判空。

具體做法是將判空方法放到 RateType 內部，然後用一個工廠方法返回一個沒有靈魂的RateType。

   // 注意，沒有返回 null
   private RateType getRateType(String span) {
       if (rateTypeList == null || span == null) return RateType.empty();
       for (RateType rt: rateTypeList) {
           if (span.equals(rt.getRateType()))
               return rt;
       }
       return RateType.empty();
   }

   private boolean isNegative() {
       return rate.compareTo(BigDecimal.ZERO) < 0;
   }

   private void setRateAndDescription(BigDecimal rate, String description) {
       setRate( rate );
       setRateDescription( description );
   }

   // 改造後
   public void changeRateByManagerV2() {
       if (CollUtil.isEmpty(rateTypeList)) return;

       if ( isNegative() ) {
           RateType y03 = getRateType("Y03");
           if ( y03.isNotEmpty() ) {
               setRateAndDescription( y03.getRate(), "近3年收益率");
           } else {
               RateType ybg = getRateType("YBG");
               if (ybg.isEmpty()) {
                   setRateAndDescription( ybg.getRate(), "成立以來收益率");
               }
           }
       }
   }

  public class RateType{
       private BigDecimal rate;
       private String rateType;

       boolean isEmpty() {
           return rate == null || rate.compareTo(BigDecimal.valueOf(-100)) == 0;
       }
       boolean isNotEmpty() {
           return !isEmpty();
       }
       // 工廠方法，避免返回null
       public static RateType empty() {
           return new RateType();
       }
   }