一个较重的代码坏味：“炫技式”的单行代码

2022-08-07 21:33:19 阅读：149 来源： 互联网

标签：map 炫技 orderInfo get 代码 null 坏味 response

风格和习惯是很重要的。

很多代码坏味都是由于不良的风格和习惯导致的，并不涉及高深复杂的技术。

有一些众所周知的代码坏味。当然，也有一些个人觉得很不好的习惯和做法。我个人就不喜欢把多行代码都“挤”到一行的写法。这种代码更像是一种“炫技式”的代码，虽然体现了一点技艺水平，但放在生产环境里并不合适：这样的代码难以理解、排查和修改起来都麻烦且耗时。我认为这种风格的代码很丑陋，这种代码所体现出来的风格和习惯很糟糕。

这样的代码俯拾即是。请看：

代码坏味举例

链式写法的误用

链式写法是把关联的多个方法调用的代码全部“挤”到一行。

究其原因，是这么写很爽。不过爽是有代价的： NPE 潜伏其中，而且报 NPE 时，定位是哪一步调用产生的 NPE 是耗时且费脑的。

如下代码所示：如果 response 为 null，或者 response.getData() 为 null，或者 response.getData().getShoppingCartDTOList() 为 null, 或者 CartResponseBuilderV2::buildCartList 的结果为 null，都会导致 NPE。链式写法仅适合于 Builder 模式，可以平行调用给一个对象的多个字段赋值，但不适合这种递进式的调用。

我之所以不喜欢链式写法，是因为排查过一个链式写法导致的 NPE。需要把链式写法拆成多行，然后单步调试是哪一行报了 NPE。很麻烦，很耗时。我不喜欢自找麻烦，痛恨时间在这种看似很爽却遗患无穷的做法中徒然地消逝：芝麻点的问题排查起来也至少是两个小时。

有两种解决技巧：1. 拆解成多个单行调用并做判空（如果变量有复用更好）； 2. 用 Optional 避免 NPE。

反例：


if (response.getData() != null && CollectionUtils.isNotEmpty(response.getData().getShoppingCartDTOList())) {
      cartList = response.getData().getShoppingCartDTOList().stream().map(CartResponseBuilderV2::buildCartList).collect(Collectors.toList());
    }

正例：

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

多个变量赋值与运算“挤”在一行

这种代码的每一个子句都会潜藏 BUG。此外，这种代码无论如何都是缺乏美感的。

实际上应该分拆成多行，每行一个赋值语句。

反例：


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

参数的构造与方法的调用糅合在一行

参数构造稍微复杂一点，要弄清楚括号的匹配都是件杀死很多脑细胞的事情。有时甚至会不小心把参数写到不正确的地方，而这时候恰巧不报错，就只能在运行时报错和排查了。我真遇到过。

如下代码所示如果 findImageIds 方法返回 null 时， addAll 方法会报 NPE。

反例：

configTrustImages.addAll(imageClient.findImageIds(ImageQueryParam.builder().imageIds(imageIds).build()));

正例：

ImageQuery query = ImageQueryParam.builder().imageIds(imageIds).build();
Set<String> imageIds = imageClient.findImageIds(query);  // 如果这个方法返回空列表而不是 null ，那就没问题。
if (CollectionUtils.isEmpty(imageIds)) {
    configTrustImages.add(imageIds);
}

另一个示例：Long.parseLong(orderInfoList.get(0).getShopId()) 就有潜在的数组越界问题。

反例：



BatchQuerySnapshotDTO requestParam =
      buildRequestParam(Long.parseLong(orderInfoList.get(0).getShopId()),
           sameUserVersionOrderMap.keySet(), exportContext);

正例：

if (CollectionUtils.isNotEmpty(orderInfoList)) {
    Long shopId = Long.parseLong(orderInfoList.get(0).getShopId()); 
    Set<String> sameUserVersionOrderKeys = sameUserVersionOrderMap.keySet();
    BatchQuerySnapshotDTO requestParam = buildRequestParam(shopId, sameUserVersionOrderKeys, exportContext);
}

滥用三目运算符

坏味是什么？

看这段逻辑，我就看了好一会，耗时耗神；
如果要添加关于 refundStatus 的新的逻辑，写在哪里？继续在里面加？
里面复杂的 refundStatus 逻辑一定是缺乏覆盖性单测的，代码不可靠。
滥用了三目运算符。这些快捷运算符本来是为了写成简洁的单行代码，可是一旦滥用，就会与其原意背道而驰。

解决方法：把构建 refundStatus 的部分抽离出来，如下所示。这样，这个方法就清晰很多，也很容易进行覆盖性单测，更可靠。

人是很容易效仿的。这样的代码看多了，你也会忍不住来上几行。

拆解单行复杂的代码

自从 Java8 有了 stream 和 lambda 表达式，程序员的自由放纵就如银河之水天上泻，奔流在屏无羁绊。lambda 表达式原本只是用于表达简短的表达式，类似于 C 语言的内联函数，但实际上被当做方法的替代者滥用，且喜欢嵌套在整个逻辑流，让整行代码理解起来很费劲、修改起来很麻烦、几乎难以单独测试。这种代码很容易滋生 BUG，很丑陋。

lambda表达式含有API调用

先来看一个比较简单的反例。如下代码所示：


response.setData(result.stream().map(
                    container -> ImageContainerDto.toDto(container, hostService.findById(container.getAgentId())))
                    .collect(Collectors.toList()));

这行代码将多个动作揉和在一起，显得不够清晰：

hostService.findById(container.getAgentId()) 是一个依赖 IO 的调用；
result.stream().map(...).collect(...) 是流的转换；
response.setData 是一个赋值的过程。

可以写成如下，更加清晰：


List<ImageContainerDto> imageContainers = StreamUtil.map(result, this::convert);
response.setData(imageContainers);

public ImageContainerDto convert(Container c) {
    Host host = hostService.findById(container.getAgentId());
    return ImageContainerDto.toDto(container, host);
}

此外，这行代码可能潜藏性能风险。当 result 条数很多时，每次都去查一遍 DB 获取 host ，有潜在性能（如果接口调用有缓存还好），且 IO 与流操作（CPU 操作）混合在一起，是不好的做法。应当将 IO 操作和 CPU 操作分离，IO 部分做成批量并发的。进一步地，写成：

List<String> agentIds = StreamUtils.map(result, Container::getAgentId);
List<Host> hosts = hostService.findBatchHosts(agentIds);
Map<String,Host> hostMap = buildMap(hosts);
 
List<ImageContainerDto> imageContainers = StreamUtils.map(result, container -> ImageContainerDto.toDto(container, hostMap.get(agentId)));
 
response.setData(imageContainers);

lambda表达式滥用和嵌套

看一看，以下代码你要花多长时间才能理解其意图？第一眼，你是理解成了 orderInfoList.forEach(orderInfo -> orderInfo.map.map.map.ifPresent) 还是理解成了 orderInfoList.forEach(orderInfo -> orderinfo.map.map).map.ifPresent , 还是理解成了 orderInfoList.forEach(orderInfo -> orderinfo.map.map.map).ifPresent ？

反例：


orderInfoList.forEach(orderInfo -> Optional.ofNullable(orderInfo.getTcExtra())
        .map(extra -> extra.get(EDU_STUDENT_INFO))
        .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())));

stream 数据流处理里的 map, ifPresent 等应用的函数如果比较复杂，应当抽离成新的函数，作为方法引用。分隔成多行之后，代码显得更自然更清晰了。

如何拆解以上的复杂语句呢？

先把容易识别的拆成子函数，减少复杂代码体的大小。比如 extra -> extra.get(EDU_STUDENT_INFO)).map(studentInfoStr -> JsonUtil.readMap((String) studentInfoStr) 还是可以识别出来的： optional.map.map.get 结构。用一个子函数来替代 getStudentInfo(OrderInfo orderInfo) ；
接着，studentInfo -> { // ... return Pair.of(studentId, version); else return null; } 是一个比较明显的 lambda 表达式滥用，应该抽离出一个子函数 buildStuPair ；

以上两步抽离出来，这个复杂代码体就比较小了。接下来，可能会有点迷惑。 ifPresent 到底是最外层的 foreach 调用的接续调用还是里层的 lambda 表达式的接续调用，add 是 ifPresent 的接续调用还是哪里的接续调用？这时候，要数括号的匹配了。不了解 ifPresent 方法的人，肯定很难继续往下拆分和理解。

话说回来，能够写出这段代码的人，对 stream, optional 之类的运用还是可以的。

正例：


public class NestedLambda {

    private static final String EDU_STUDENT_INFO = "student_info";
    private static final String STUDENT_ID = "student_id";
    private static final String VERSION = "version";
    private static final String ID_CARD_NO = "id_card";

    public void nestedLambda() {
        List<OrderInfo> orderInfoList = Lists.newArrayList();
        Map<Pair, List<String>> sameVersionOrderMap = new HashMap();
        orderInfoList.forEach(orderInfo -> buildStudentPair(orderInfo).ifPresent(
                idVersionPair -> setOrderMap(idVersionPair, orderInfo, sameVersionOrderMap)));

    }

    private void setOrderMap(Pair idVersionPair, OrderInfo orderInfo, Map<Pair, List<String>> sameVersionOrderMap) {
        sameVersionOrderMap
                .computeIfAbsent(idVersionPair, k -> Lists.newArrayList())
                .add(orderInfo.getOrderNo());
    }

    private Optional<Pair> buildStudentPair(OrderInfo orderInfo) {
        return getStudentInfo(orderInfo).map(this::buildStuPair);
    }

    private Optional<Map> getStudentInfo(OrderInfo orderInfo) {
        return Optional.ofNullable(orderInfo.getTcExtra())
                .map(extra -> extra.get(EDU_STUDENT_INFO))
                .map(studentInfoStr -> JsonUtil.readMap((String) studentInfoStr));
    }

    private Pair buildStuPair(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;
        }
    }
}

小结

很多编程语言赋予了程序员很大的灵活性。然而，如果程序员不克制冲动的欲望，自由放纵地写代码，就很容易写出看上去很爽但实际上很丑陋的代码。

代码都挤在一行，也会感觉不舒服的。想想你挤地铁的感受，考虑下代码的感受。

标签：map,炫技,orderInfo,get,代码,null,坏味,response
来源： https://www.cnblogs.com/lovesqcc/p/16559923.html

本站声明： 1. iCode9 技术分享网（下文简称本站）提供的所有内容，仅供技术学习、探讨和分享；
2. 关于本站的所有留言、评论、转载及引用，纯属内容发起人的个人观点，与本站观点和立场无关；
3. 关于本站的所有言论和文字，纯属内容发起人的个人观点，与本站观点和立场无关；
4. 本站文章均是网友提供，不完全保证技术分享内容的完整性、准确性、时效性、风险性和版权归属；如您发现该文章侵犯了您的权益，可联系我们第一时间进行删除；
5. 本站为非盈利性的个人网站，所有内容不会用来进行牟利，也不会利用任何形式的广告来间接获益，纯粹是为了广大技术爱好者提供技术内容和技术思想的分享性交流网站。

ICode9