一个 Java Optional + Stream 重构的小例子

public Optional<MyLog> lastLog(Collection<Long> shopIds) {
    List<Long> allAppIds = StreamUtils.map(appDao.findAll(), App::getId);
    List<MyLog> logs = new ArrayList<>();
    shopIds.forEach(sid -> {
        allAppIds.forEach(aid -> {
            MyLog lastLog = logDao.findLast(aid, sid, null, LocalDate.now().plusDays(1));
            if (Objects.nonNull(lastLog)) {
                logs.add(lastLog);
            }
        });
    });
    if (CollectionUtils.isEmpty(logs)) {
        return Optional.empty();
    }
    logs.sort(Comparator.comparing(MyLog::getTime));
    return Optional.of(Iterables.getLast(logs));
}


public Optional<MyLog> lastLog(Collection<Long> shopIds) {
    if (shopIds.isEmpty()) {
        return Optional.empty();
    }
    var allAppIds = appDao.findAllId();
    var end = LocalDate.now().plusDays(1);
    return shopIds.stream()
            .flatMap(shopId -> allAppIds.stream().flatMap(appId -> logDao.findLast(appId, shopId, end).stream()))
            .max(Comparator.comparing(MyLog::getTime));
}

我们用的 java 11 ，部分方法 java 8 可能没有

类名和方法名有做部分处理

原来的方法不知道为什么主动用了 optional 但还是写成这么一大坨

调整后的 early return 其实也能合并到 optional 里串起来，不过想想还是算了，反倒显得更麻烦

另外这个 max 是 idea 自动提示的调整，一开始是照着原方法的思路写成 sorted + findFirst

idea 很多 warning 都可以很方便的 alt + enter 直接重构掉

copilot 锐评： Overall, the refactored implementation is more concise and readable than the original implementation, and makes use of functional programming constructs to achieve the same result. It also avoids the use of nested loops, which can be harder to read and understand.

anonydmer

2023-06-20 12:29:17 +08:00

看起来就是对一个 appID 和一个 shopID 两个 id 集合做了笛卡尔集的遍历查最后一条日志，在不考虑楼上几位说的性能的前提下我更乐意写成：

shopIDs.stream()
.flatMap( s -> appIDs.stream().map( a -> new Pair(s, a)) )
.map(p -> logDAO.findLast(p.first(), p.second(), end)
.max(xxx)

第一步单独构建两个 id 集合的笛卡尔集（其实用个别的库可能比 stream 更方便）
第二步遍历取数据，这一步其实是可以利用 Stream 的并行计算的
第三步 max

TWorldIsNButThis

2023-06-20 13:58:05 +08:00

回复一下，首先 findLastLog 并不是一个 mysql 简单查询，本身有其特有逻辑，它甚至不是 mysql

然后这个重构也并不是意在调整设计方案，而是调整查询的业务逻辑后顺手精简一下调用处代码

最后”不要循环调用查询“这类教条并不是放之四海而皆准的铁律，一切都要由结合业务做评估，对于业务代码，可读性永远是默认第一位的，性能优化要以实际表现为准。

hhhh115

2023-06-20 14:54:33 +08:00

@TWorldIsNButThis #13 '不要循环调用查询' 这个并不怎么赞同吧，如果循环中写查询，每一个循环都要和数据库传输数据。有点儿像在，for 循环里进行 http 请求。
感觉如果是，正常 for 循环处理数据（不过网络，磁盘）的话，可读性确实更重要些。查询的话，先把数据处理好给数据库，然后一次查出所有出局，可能会好点吧。可读性差的话，多写点注释就好了吧

2013881276

2023-06-20 16:40:18 +08:00

@anonydmer 补充完整了点：public Optional<MyLog> lastLog(final Collection<Long> shopIds) {
return Optional.ofNullable(CollectionUtil.isEmpty(shopIds) ? null : shopIds)
.map(shopIdList -> Pair.of(shopIdList, appDao.findAllId()))
.map(pair -> pair.getFirst()
.stream()
.flatMap(shopId -> {
final LocalDate end = LocalDate.now().plusDays(1);
return pair.getSecond().stream().flatMap(appId -> logDao.findLast(appId, shopId, end).stream());
})
.max(Comparator.comparing(MyLog::getTime)))
.orElse(Optional.empty());
}