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

[rhoasneg]

今天在对我们企业项目里进行 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 个）想问下各位是否也有在项目中这样写的经验？

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

• 这是一段查询分页数据，这里前端传的是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;
}

唉，无奈

[kdd0063]

这看你从什么角度看问题了。如果从 tech lead 的角度来看，项目代码的风格统一，可读性与可维护性是第一位的，很多时候甚至比性能收益更重要。这段 PR 带来了更差的可读性，可维护性，因为数据量很小所以根本不会体现多么高的性能收益，几乎属于纯粹的炫技行为，这几个点一叠加，必定是要打回重改的。

[zhujinliang]

过早优化是万恶之源

[passive]

简单干净低耦合的函数，做好边界场景的测试，里面写成花都不管。

[nagisaushio]

第二段代码。。
不懂位运算的人看不懂
懂位运算的人看了觉得写得烂
综上肯定会被打

[rhoasneg]

@kdd0063 是的，就我个人角度看的话，这种写法对于人来说，是不友好的(毕竟代码是给人看的)，并且后续在定位问题上也会很麻烦
@zhujinliang 其实要说优化的话，我感觉似乎也没优化到点上吧，因为需要一次返回多条数据，所以他实际上在返回的时候，还需要对每条数据进行转换

[rhoasneg]

@passive 这种函数输出的结果，如果线上出现相关异常，定位会比较麻烦吧

[rhoasneg]

@nagisaushio hhhhhhh

[kdd0063]

@rhoasneg 这种 code ，除非是不这么写服务性能直接下降一个量级才能容忍，而且起码必须有详尽的注释，这些条件都不符合可以直接毙了

[orangie]

就算想用位运算，java 有 BitSet 可以用，不应该自己用 int 写。

[billlee]

在 C 里面用位运算传 flag 是基本操作，unix 系统调用都是这么传的，而且这个用法就是适合列表值不多的情况。就是他这样封装一下，就显得没有什么意义了。

[akira]

大部分情况下可维护性是第一位，特别是业务逻辑部分。

不是性能不重要，是性能优化应该是系统性的

[yolee599]

C 语言常规操作，可能这个同事之前写 C 的吧

[xiangyuecn]

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]

cpu 时间明显比内存金贵的多，又不是极致压缩的场景，没必要搞这种花里胡哨的东西。

[jeesk]

这种一般我们用来保存状态。 如果特别是独立的多个状态。如果是 id, 感觉没啥必要。 没可读性。浪费

[jeesk]

@xiangyuecn 这种位运算每个数字就是一个状态， 假设有 10 个状态，是不可能在数据库放 10 个状态的， 这个时候，这种方式来解决确实是一个不错的方案。

[tool2d]

我看 google grpc 代码里一大堆位运算，也没啥问题。把注释写清楚就可以了。

有太多的代码，比这个要糟糕。

[512357301]

id 用位运算感觉纯粹浪费空间，到最后变成 1 2 4 8 16 32 这样的十进制数，中间的空位都被浪费了，随便十几个 ID 之后数字就很大了，存储、记忆都不方便。
不知道我说的这个是不是你说的位运算，反正我们这有研发这么干的，我当时惊为天人。

[hunterzhang86]

大家一起开发的项目里面，可读性会更重要一些，主要是你写的代码如果很难理解，大家配合起来就很困难。

[netabare]

不知道在 TDD 的项目里，能不能把一个函数当成黑箱，不管是位运算还是别的做法，只要给定了明确的说明和期望，并且写了测试来检查函数的正确性，就可以不管它内部的具体实现是否炫技什么的。

不过我也不用 TDD 所以这种情况确实感觉可读性更重要，至少半年后自己回过头再检查的时候能读懂。

[post90sraccoon]

可读性重要的吗，代码是给人看的

[zhady009]

又不是写内核，这种性能带来的提升在应用级上没啥意义

[adoal]

位运算都成了高级技巧…

[hez2010]

一般位运算方式存储数值是为了节省空间，但是 Java 压根连值类型都没有，走哪都得 box ，这 box 一下多出来的大小都比你省出来的空间远大的多。
所以我的评价是在 Java 里这么写没用，但是在其它语言里有用。

[rhoasneg]

@tool2d 你可以看下我新加的附言内容，可能和你理解有点不同。。。

[tool2d]

@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]

@tool2d 你这种代表状态值的使用是正常的，但是他这种感觉就是刻意在埋坑的