[代码拯救行动] 10月份 代码检视案例+重构

代码检视问题总结

案例一

首先来看一段代码

        //根据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();
       }
   }