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

关于可读性与高级技巧之间的折衷

  •  2
     
  •   rhoasneg · 2023-11-25 17:13:29 +08:00 · 5318 次点击
    这是一个创建于 365 天前的主题,其中的信息可能已经有所发展或是发生改变。

    今天在对我们企业项目里进行 CR ,其中发现有位同事在保存 id 列表时采用位运算的方式,将 int 类型的列表转成 二进制值,代码:

        private int getFlag(List<Integer> flags) {
            int result = 0;
            if (flags == null) {
                return 0;
            }
            for (Integer flag : flags) {
                result |= 1 << flag;
            }
            return result;
        }
    

    在取出列表时,则:

        private List<Integer> toFlagList(int flag) {
            List<Integer> result = new ArrayList<>();
            for (int i = 0; i < 32; i++) {
                if (((flag >>> i) & 1) == 1) {
                    result.add(i);
                }
            }
            return result;
        }
    

    这种写法,我个人觉得,在存储数值较多的列表时还不错,就是代码可读性不好(对于平时不怎么使用到位运算的同事),此外,这种时间换空间的方式,是否收效甚微呢?( id 列表值不多,就一个下拉框,在可预见的未来应该也不会超过 10 个)想问下各位是否也有在项目中这样写的经验?

    第 1 条附言  ·  362 天前

    昨天维护了这位同事的代码(刚离职),脑壳疼,简单贴些片段吧:

    • 这是一段查询分页数据,这里前端传的是id数组,这里使用 getFlag() 转换还可以理解
    alarmInfoService.getSimpleInfoPage(param.getPerPageCount(), param.getCurPage(), param.getKeyWord(), getFlag(param.getScenesList()), cameraIds);
    
    • 由于他把逻辑都写在了 mapper层,就不贴 ServiceImpl 的内容了,这里是 mapper 层的逻辑...
    default IPage<AlarmInfoDO> selectPage(int count, int cur, String keyword, int flag, List<String> cameraIds) {
        ....
        if (flag != 0) {
            List<Integer> flags = getAllFlags(flag);
            for (int i = 0; i < flags.size(); i++) {
                if (i != 0) {
                    sql.append(" or ");
                } else {
                    sql.append(" and (");
                }
                sql.append(String.format("((type & %d) > 0)", flags.get(i)));
            }
            sql.append(")");
        }
        ....
    }
    
    default List<Integer> getAllFlags(int flag) {
        List<Integer> flags = new ArrayList<>();
        for (int i = 0; i < 32; i++) {
            if (((flag >>> i) & 1) == 1) {
                flags.add(1 << i);
            }
        }
        return flags;
    }
    

    唉,无奈

    27 条回复    2023-11-28 10:55:39 +08:00
    kdd0063
        1
    kdd0063  
       2023-11-25 17:19:11 +08:00
    这看你从什么角度看问题了。如果从 tech lead 的角度来看,项目代码的风格统一,可读性与可维护性是第一位的,很多时候甚至比性能收益更重要。这段 PR 带来了更差的可读性,可维护性,因为数据量很小所以根本不会体现多么高的性能收益,几乎属于纯粹的炫技行为,这几个点一叠加,必定是要打回重改的。
    zhujinliang
        2
    zhujinliang  
       2023-11-25 17:21:16 +08:00 via iPhone
    过早优化是万恶之源
    passive
        3
    passive  
       2023-11-25 17:25:05 +08:00 via Android
    简单干净低耦合的函数,做好边界场景的测试,里面写成花都不管。
    nagisaushio
        4
    nagisaushio  
       2023-11-25 17:26:45 +08:00 via Android   ❤️ 1
    第二段代码。。
    不懂位运算的人看不懂
    懂位运算的人看了觉得写得烂
    综上肯定会被打
    rhoasneg
        5
    rhoasneg  
    OP
       2023-11-25 17:31:09 +08:00
    @kdd0063 是的,就我个人角度看的话,这种写法对于人来说,是不友好的(毕竟代码是给人看的),并且后续在定位问题上也会很麻烦
    @zhujinliang 其实要说优化的话,我感觉似乎也没优化到点上吧,因为需要一次返回多条数据,所以他实际上在返回的时候,还需要对每条数据进行转换
    rhoasneg
        6
    rhoasneg  
    OP
       2023-11-25 17:33:20 +08:00
    @passive 这种函数输出的结果,如果线上出现相关异常,定位会比较麻烦吧
    rhoasneg
        7
    rhoasneg  
    OP
       2023-11-25 17:35:10 +08:00
    @nagisaushio hhhhhhh
    kdd0063
        8
    kdd0063  
       2023-11-25 18:08:48 +08:00
    @rhoasneg 这种 code ,除非是不这么写服务性能直接下降一个量级才能容忍,而且起码必须有详尽的注释,这些条件都不符合可以直接毙了
    orangie
        9
    orangie  
       2023-11-25 18:21:26 +08:00   ❤️ 3
    就算想用位运算,java 有 BitSet 可以用,不应该自己用 int 写。
    billlee
        10
    billlee  
       2023-11-25 18:30:16 +08:00 via Android
    在 C 里面用位运算传 flag 是基本操作,unix 系统调用都是这么传的,而且这个用法就是适合列表值不多的情况。就是他这样封装一下,就显得没有什么意义了。
    akira
        11
    akira  
       2023-11-25 18:30:52 +08:00
    大部分情况下可维护性是第一位,特别是业务逻辑部分。

    不是性能不重要,是性能优化应该是系统性的
    yolee599
        12
    yolee599  
       2023-11-25 20:01:19 +08:00 via Android
    C 语言常规操作,可能这个同事之前写 C 的吧
    xiangyuecn
        13
    xiangyuecn  
       2023-11-25 20:33:30 +08:00
    bit="";s=[];for(var i=0;i<32;i++) s.push('0x'+parseInt("1"+bit,2).toString(16)),bit+="0"; console.log(s.join("\n"))

    0x1
    0x2
    0x4
    0x8
    0x10
    0x20
    0x40
    0x80
    0x100
    0x200
    0x400
    0x800
    0x1000
    0x2000
    0x4000
    0x8000
    0x10000
    0x20000
    0x40000
    0x80000
    0x100000
    0x200000
    0x400000
    0x800000
    0x1000000
    0x2000000
    0x4000000
    0x8000000
    0x10000000
    0x20000000
    0x40000000
    0x80000000

    没发现还挺规律的
    night98
        14
    night98  
       2023-11-25 22:10:44 +08:00
    cpu 时间明显比内存金贵的多,又不是极致压缩的场景,没必要搞这种花里胡哨的东西。
    jeesk
        15
    jeesk  
       2023-11-25 22:29:25 +08:00
    这种一般我们用来保存状态。 如果特别是独立的多个状态。如果是 id, 感觉没啥必要。 没可读性。浪费
    jeesk
        16
    jeesk  
       2023-11-25 22:30:49 +08:00
    @xiangyuecn 这种位运算每个数字就是一个状态, 假设有 10 个状态,是不可能在数据库放 10 个状态的, 这个时候,这种方式来解决确实是一个不错的方案。
    tool2d
        17
    tool2d  
       2023-11-25 23:03:45 +08:00
    我看 google grpc 代码里一大堆位运算,也没啥问题。把注释写清楚就可以了。

    有太多的代码,比这个要糟糕。
    512357301
        18
    512357301  
       2023-11-25 23:09:42 +08:00 via Android
    id 用位运算感觉纯粹浪费空间,到最后变成 1 2 4 8 16 32 这样的十进制数,中间的空位都被浪费了,随便十几个 ID 之后数字就很大了,存储、记忆都不方便。
    不知道我说的这个是不是你说的位运算,反正我们这有研发这么干的,我当时惊为天人。
    hunterzhang86
        19
    hunterzhang86  
       2023-11-26 06:58:18 +08:00 via iPhone
    大家一起开发的项目里面,可读性会更重要一些,主要是你写的代码如果很难理解,大家配合起来就很困难。
    netabare
        20
    netabare  
       364 天前 via iPhone   ❤️ 1
    不知道在 TDD 的项目里,能不能把一个函数当成黑箱,不管是位运算还是别的做法,只要给定了明确的说明和期望,并且写了测试来检查函数的正确性,就可以不管它内部的具体实现是否炫技什么的。

    不过我也不用 TDD 所以这种情况确实感觉可读性更重要,至少半年后自己回过头再检查的时候能读懂。
    post90sraccoon
        21
    post90sraccoon  
       364 天前
    可读性重要的吗,代码是给人看的
    zhady009
        22
    zhady009  
       364 天前 via iPhone
    又不是写内核,这种性能带来的提升在应用级上没啥意义
    adoal
        23
    adoal  
       364 天前 via iPhone
    位运算都成了高级技巧…
    hez2010
        24
    hez2010  
       364 天前
    一般位运算方式存储数值是为了节省空间,但是 Java 压根连值类型都没有,走哪都得 box ,这 box 一下多出来的大小都比你省出来的空间远大的多。
    所以我的评价是在 Java 里这么写没用,但是在其它语言里有用。
    rhoasneg
        25
    rhoasneg  
    OP
       362 天前
    @tool2d 你可以看下我新加的附言内容,可能和你理解有点不同。。。
    tool2d
        26
    tool2d  
       362 天前
    @rhoasneg sql 写的有点奇怪,理论上运算一次就可以了,不需要拼接 sql 。

    可能是编程领域不一样,我代码里大量使用位运算,比如下面这种:

    enum {
    PERM_ACTIVE = (1<<0), // 帐号被激活状态,
    PERM_CHANNEL = (1<<1), // 添加/删除频道权限
    PERM_PUB = (1<<2), // 发布文章权限
    PERM_REVIEW = (1<<3), // 审批权限
    PERM_DEL = (1<<4), // 删除文章权限
    };
    rhoasneg
        27
    rhoasneg  
    OP
       362 天前
    @tool2d 你这种代表状态值的使用是正常的,但是他这种感觉就是刻意在埋坑的
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   1040 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 22ms · UTC 23:08 · PVG 07:08 · LAX 15:08 · JFK 18:08
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.