V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
TWorldIsNButThis
V2EX  ›  Java

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

  •  
  •   TWorldIsNButThis · 2023-06-20 11:28:39 +08:00 · 2522 次点击
    这是一个创建于 521 天前的主题,其中的信息可能已经有所发展或是发生改变。
    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.

    21 条回复    2023-10-22 23:23:50 +08:00
    allenzhangSB
        1
    allenzhangSB  
       2023-06-20 11:56:06 +08:00   ❤️ 2
    重构后还是在循环中查 db, 这个是最应该重构掉的
    yazinnnn
        2
    yazinnnn  
       2023-06-20 12:03:00 +08:00
    如果是查库的话, 为啥不直接写 sql?
    oneisall8955
        3
    oneisall8955  
       2023-06-20 12:23:04 +08:00 via Android
    日志表分库分表了吗?循环里面查 db 有影响
    anonydmer
        4
    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
    tulongtou
        5
    tulongtou  
       2023-06-20 12:30:41 +08:00
    循环里面操作数据库真是最应该优化的地方
    oldshensheep
        6
    oldshensheep  
       2023-06-20 12:34:10 +08:00
    你写代码是不是一句 SQL 不用写,全用 Java 处理

    App 表也不知道里面存的什么东西,没改前的 appDao.findAll(),我也是惊呆了。
    xuanbg
        7
    xuanbg  
       2023-06-20 12:38:40 +08:00
    换我写的话,一句 sql 直接搞定。。。
    nikenidage1
        8
    nikenidage1  
       2023-06-20 13:00:34 +08:00   ❤️ 1
    脑壳疼
    如果你用 C#,大概就是这样

    shopIds.Where(log => log != null).OrderBy(log => log.Time).LastOrDefault();
    liprais
        9
    liprais  
       2023-06-20 13:00:41 +08:00
    没做过 profile 吧
    循环里面查数据库
    StevenQAQ
        10
    StevenQAQ  
       2023-06-20 13:18:15 +08:00
    我理解你把 logDao 的查询提出来会好点,比如用日期范围查询一下,如果数据量很大就把 aid 和 sid 的范围数据也传进去。比循环查效率好很多。
    season8
        11
    season8  
       2023-06-20 13:38:49 +08:00
    如果表设计得合理,直接按 shopIds 查询不就可以了,都不需要用 appId ,也不需要循环查询,一个 sql 就解决了
    imv2er
        12
    imv2er  
       2023-06-20 13:40:31 +08:00
    我见了我深恶痛绝的返回 Optional<xxxx>
    TWorldIsNButThis
        13
    TWorldIsNButThis  
    OP
       2023-06-20 13:58:05 +08:00
    回复一下,首先 findLastLog 并不是一个 mysql 简单查询,本身有其特有逻辑,它甚至不是 mysql

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

    最后”不要循环调用查询“这类教条并不是放之四海而皆准的铁律,一切都要由结合业务做评估,对于业务代码,可读性永远是默认第一位的,性能优化要以实际表现为准。
    TWorldIsNButThis
        14
    TWorldIsNButThis  
    OP
       2023-06-20 14:01:50 +08:00
    @anonydmer 如果是 kotlin ,我会这么写,而且构造完 pair 在 map 中使用的时候必须是解构声明而不是 first second 取值

    但对于 java 还是直接一些为好
    hhhh115
        15
    hhhh115  
       2023-06-20 14:54:33 +08:00
    @TWorldIsNButThis #13 '不要循环调用查询' 这个并不怎么赞同吧,如果循环中写查询,每一个循环都要和数据库传输数据。有点儿像在,for 循环里进行 http 请求。
    感觉如果是,正常 for 循环处理数据(不过网络,磁盘)的话,可读性确实更重要些。查询的话,先把数据处理好给数据库,然后一次查出所有出局,可能会好点吧。可读性差的话,多写点注释就好了吧
    Slurp
        16
    Slurp  
       2023-06-20 15:24:56 +08:00
    @nikenidage1 扯淡,你这个逻辑都不对应。flatMap 哪去了?
    2013881276
        17
    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());
    }
    mmdsun
        18
    mmdsun  
       2023-06-20 16:44:13 +08:00
    排序 Comparator.comparing(MyLog::getTime)为空会报错吧?
    可以换成一个 Comparator.Comparator.nullsFirst()或 Comparator.nullsLast()
    mmdsun
        19
    mmdsun  
       2023-06-20 16:51:26 +08:00
    @mmdsun 推荐用 Rxjava 或者 Reactor Project (或者用像 Spring Webflux 这种) ,Java 内置的操作符太少不够用。写起来也难看。
    zengguibo
        20
    zengguibo  
       2023-07-02 13:06:27 +08:00
    最应该优化的是在循环调用查询,正常是一次将数据取出来,在内存中比对
    forgottencoast
        21
    forgottencoast  
       2023-10-22 23:23:50 +08:00
    @nikenidage1
    真的,和 C#的 Linq 对比起来 Java 的 Stream 太难用了。
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   2758 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 19ms · UTC 12:06 · PVG 20:06 · LAX 04:06 · JFK 07:06
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.