要用一种精致的态度去写代码,才能写出优美而牢固的代码。

本文主要从日常代码中摘录一些不良的写法。这些不良的写法会扰乱清晰的主流程,淹没重要的业务逻辑,使得代码语义难以理解和修改。


代码坏味

超长链式

超长链式的坏处: 1. getXXX() 重复出现; 2. 容易 NPE ;3. 非常丑


   if (response.getData() != null && CollectionUtils.isNotEmpty(response.getData().getShoppingCartDTOList())) {

      // 出参转换

      cartList = response.getData().getShoppingCartDTOList().stream().map(CartResponseBuilderV2::buildCartList).collect(Collectors.toList());

    }

写成下面较好:

像 StreamUtil 这样的工具类应该抽离出来,方便其他同学更好地写代码。


T data = response.getData();

if (data != null && CollectionUtils.isNotEmpty(data.getShoppingCartDTOList())) {

  cartList = StreamUtil.map(data.getShoppingCartDTOList(), CartResponseBuilderV2::buildCartList);

}


不够的单测####

一个很大的入口方法 A ,调用了 B 和 C , 只对 A 做了单测,认为 B 和 C 自然覆盖。

坏处:1. 当 B 和 C 添加新的代码路径时,要写单测非常困难; 2. A 的单测代码不一定覆盖 B,C 的所有重要情况。

只要 B 和 C 含有比较重要的业务点,尤其是比较复杂的业务逻辑时,就应该加以单测。


未抽离的变量

很容易就顺手写出这样的代码: 把所有一坨东西扔到一个语句,就像把所有一坨代码扔到一个方法里一样。


long discounts =

                    Optional.ofNullable(orderItemInfo).map(OrderItemInfo::getPrice).orElse(0L)

                    - Optional.ofNullable(orderItemInfo).map(OrderItemInfo::getPcOrderItemPrice)

                        .map(PcOrderItemPrice::getUnitPrice).orElse(0L);

应该把变量抽离处理:


Long originPrice = Optional.ofNullable(orderItemInfo).map(OrderItemInfo::getPrice).orElse(0L);

Long unitPrice = Optional.ofNullable(orderItemInfo).map(OrderItemInfo::getPcOrderItemPrice)

                        .map(PcOrderItemPrice::getUnitPrice).orElse(0L);

long discounts = originPrice - unitPrice

这种也是暗藏危机:


  BatchQuerySnapshotDTO requestParam =

        buildRequestParam(Long.parseLong(orderInfoList.get(0).getShopId()),

                          sameUserVersionOrderMap.keySet(), exportContext);

这里取 shopId 用了 Long.parseLong(orderInfoList.get(0).getShopId()) 而且还写在方法调用参数列表里,这样有两个坏处:

  1. orderInfoList 如果为空列表或者后续被改成空列表,就会报越界错误;

  2. 取值逻辑放在方法参数里,导致方法调用复杂难看。


未抽离的函数

看下面这段代码:


private List<Get> buildGets(List<String> rowKeyList, String cf, List<String> columns, List<String> columnPrefixFilters) {

        return StreamUtil.map(

            rowKeyList,

            rowKey -> {

                String rowKeyNotEmpty = (rowKey == null ? "null" : rowKey);

                Get get = new Get(Bytes.toBytes(rowKeyNotEmpty));

                if (columns != null && !columns.isEmpty()) {

                    for (String col: columns) {

                        get.addColumn(Bytes.toBytes(cf), Bytes.toBytes(col));

                    }

                }

                MultipleColumnPrefixFilter multipleColumnPrefixFilter = buildMultipleColumnPrefixFilter(columnPrefixFilters);

                if (multipleColumnPrefixFilter != null) {

                    get.setFilter(multipleColumnPrefixFilter);

                }

                return get;

            });

    }

其中 rowKey -> { // code... } 中的 块 code 是没有抽离出来的,这样写的话,很容易就忽视对里面代码的单测,从而埋伏潜在的BUG。应该将这样的块 code 抽离成一个函数,并加以单测。


    private List<Get> buildGetsV2(List<String> rowKeyList, String cf, List<String> columns, List<String> columnPrefixFilters) {

        return StreamUtil.map(rowKeyList, rowKey -> buildGet(rowKey, cf, columns, columnPrefixFilters) );

    }



    private Get buildGet(String rowKey, String cf, List<String> columns, List<String> columnPrefixFilters) {

        String rowKeyNotEmpty = (rowKey == null ? "null" : rowKey);

        Get get = new Get(Bytes.toBytes(rowKeyNotEmpty));

        if (columns != null && !columns.isEmpty()) {

            for (String col: columns) {

                get.addColumn(Bytes.toBytes(cf), Bytes.toBytes(col));

            }

        }

        MultipleColumnPrefixFilter multipleColumnPrefixFilter = buildMultipleColumnPrefixFilter(columnPrefixFilters);

        if (multipleColumnPrefixFilter != null) {

            get.setFilter(multipleColumnPrefixFilter);

        }

        return get;

    }


不够优美

这段代码并没有问题,但是不够优美。




  public Boolean kdtIdsLimit(LimitStageEnum limitStageEnum, Set<Long> kdtIds) {

    for (Long kdtId : kdtIds) {

      Map<String, String> features = Maps.newHashMap();

      features.put("stage", limitStageEnum.getValue());

      features.put("kdt_id", kdtId.toString());

      TrafficLimitDecision decision = programmingTrafficLimitAdapter.apply(features);

      if (decision.shouldLimit()) {

        return true;

      }

    }

    return false;

  }

优美的写法是什么 ? 该分离的关注点分离出来,努力寻求最简洁的方式表达。

这段代码含有两个语义:1. 对一个 shopId 判断是否限流; 2. 如果有一个限流,则所有都要限流。一个是单店铺的限流判断的实现,一个是多个店铺的限流匹配规则。应该分离开。




  public Boolean shopIdsLimit(LimitStageEnum limitStageEnum, Set<Long> shopIds) {

    return shopIds.stream().anyMatch(this::shouldLimit);

  }

 

  private boolean shouldLimit(Long shopId) {

    Map<String, String> features = Maps.newHashMap();

    features.put("stage", limitStageEnum.getValue());

    features.put("shop_id", shopId.toString());

    TrafficLimitDecision decision = programmingTrafficLimitAdapter.apply(features);

    return decision.shouldLimit();

  }


复杂逻辑

看下面这段代码,三个 map 的逻辑很复杂,放在里面:

坏处:1. 复杂逻辑无法单测,很容易出 BUG ,很容易 NPE 或 各种异常; 2. 流程逻辑不清晰。

你能看懂这里面写的是什么吗 ?


private Map<Pair<Long, Integer>, List<String>> aggregateSameUserVersionOrder(

      List<OrderInfo> orderInfoList) {

    Map<Pair<Long, Integer>, List<String>> sameVersionOrderMap = Maps.newHashMap();

 

    orderInfoList.forEach(orderInfo -> Optional.ofNullable(orderInfo.getTcExtra())

        .map(extra -> extra.get(EDU_STUDENT_INFO))

        // extraMap中存的是String

        .map(studentInfoStr -> JsonUtil.readMap((String) studentInfoStr)).map(studentInfo -> {

          Long studentId = (Long) studentInfo.get(STUDENT_ID);

          Integer version = (Integer) studentInfo.get(VERSION);

          if (studentInfo.containsKey(ID_CARD_NO) && studentId != null && version != null) {

            return Pair.of(studentId, version);

          } else {

            return null;

          }

        }).ifPresent(

            idVersionPair -> sameVersionOrderMap

                .computeIfAbsent(idVersionPair, k -> Lists.newArrayList())

                .add(orderInfo.getOrderNo())));

    return sameVersionOrderMap;

  }

这样绕口的代码,最好用比较平易的方式表达更佳。 不要滥用语言特性。




private Map<Pair<Long, Integer>, List<String>> aggregateSameUserVersionOrder(List<OrderInfo> orderInfoList) {



    Map<Pair<Long, Integer>, List<String>> sameUserVersionOrder = new HashMap<>();



    for (OrderInfo orderInfo: orderInfoList) {

      Pair<Long, Integer> p = generatePair(orderInfo.getTcExtra());

      if (p == null) { continue; }

      List<String> orderNoList = sameUserVersionOrder.put(p, sameUserVersionOrder.getOrDefault(p, new ArrayList<>());

      orderNoList.add(orderInfo.getOrderNo());

    }

    return sameUserVersionOrder;

  }



  private Pair<Long, Integer> generatePair(Map<String,Object> tcExtra) {

    if (tcExtra == null || !tcExtra.containsKey(EDU_STUDENT_INFO)) {

      return null;

    }



    Map studentInfoMap = JsonUtil.readMap(tcExtra.get(EDU_STUDENT_INFO).toString());

    Long studentId = (Long) studentInfoMap.get(STUDENT_ID);

    Integer version = (Integer) studentInfoMap.get(VERSION);



    if (studentInfoMap.containsKey(ID_CARD_NO) && studentId != null && version != null) {

      return Pair.of(studentId, version);

    } else {

      return null;

    }

  }




小结

代码是逻辑与表达的艺术。

代码编写,就像一门手工艺。要打造出优秀的作品,良好的习惯和细节是关键。改掉不良的代码习惯,坚持清爽整洁之道,才能让代码更优美而牢固。


什么是好的代码习惯 ? 两个小原则:

  1. 拆解。凡有比较复杂的逻辑或关注点,分离细小可复用的关注点, 分离出来作为变量或函数。

  2. 简洁。始终追求简洁易懂的表达。


相关文章: