代码的味道

时间 2019-11-06

标签代码味道繁體版

原文原文链接

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

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

app

代码坏味

超长链式

超长链式的坏处： 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 这样的工具类应该抽离出来，方便其余同窗更好地写代码。ui

T data = response.getData();
if (data != null && CollectionUtils.isNotEmpty(data.getShoppingCartDTOList())) {
  cartList = StreamUtil.map(data.getShoppingCartDTOList(), CartResponseBuilderV2::buildCartList);
}

不够的单测

一个很大的入口方法 A ，调用了 B 和 C ，只对 A 作了单测，认为 B 和 C 天然覆盖。this

坏处：1. 当 B 和 C 添加新的代码路径时，要写单测很是困难； 2. A 的单测代码不必定覆盖 B,C 的全部重要状况。code

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

未抽离的变量

很容易就顺手写出这样的代码：把全部一坨东西扔到一个语句，就像把全部一坨代码扔到一个方法里同样。ci

long discounts =
                    Optional.ofNullable(orderItemInfo).map(OrderItemInfo::getPrice).orElse(0L)
                    - Optional.ofNullable(orderItemInfo).map(OrderItemInfo::getPcOrderItemPrice)
                        .map(PcOrderItemPrice::getUnitPrice).orElse(0L);

应该把变量抽离处理:get

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()) 并且还写在方法调用参数列表里，这样有两个坏处：

orderInfoList 若是为空列表或者后续被改为空列表，就会报越界错误；
取值逻辑放在方法参数里，致使方法调用复杂难看。

未抽离的函数

看下面这段代码：

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;
    }
  }

小结

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

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

什么是好的代码习惯？两个小原则：

拆解。凡是有比较复杂的逻辑或关注点，分离细小可复用的关注点，分离出来做为变量或函数。
简洁。始终追求简洁易懂的表达。