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

写程序这么精简真的好吗?

  •  5
     
  •   wsy190 · 74 天前 · 15433 次点击
    这是一个创建于 74 天前的主题,其中的信息可能已经有所发展或是发生改变。

    我有一个同事写代码特别精简。。如:

    public OutVoGlobal list(BatteryOrderDTO dto, UserInfo user) {

    return new OutVoGlobal(EnumRetCode.SUCCESS).setData(orderMapper.list(dto.setBelong(user.getUserNo())));

    }
    

    之后这段代码有一些问题,让我来修改这段代码。。我就觉得这段代码的可读性特别的差。昨天和他讨论了一下,他觉得代码行数多影响阅读,他这样他看起来很舒服。以下是我加了判断后的:

    public OutVoGlobal list(BatteryOrderDTO dto, UserInfo user) {

        OutVoGlobal outVoGlobal=new OutVoGlobal(EnumRetCode.SUCCESS);
        SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
        if(!StringUtils.isEmpty(dto.getStartTime())){
            try {
                sdf.parse(dto.getStartTime());
                dto.setStartTime(dto.getStartTime()+" 00:00:00");
            } catch (ParseException e) {
                dto.setStartTime("");
            }
        }
        if(!StringUtils.isEmpty(dto.getEndTime())){
            try {
                sdf.parse(dto.getEndTime());
                dto.setEndTime(dto.getEndTime()+" 23:59:59");
            } catch (ParseException e) {
                dto.setEndTime("");
            }
        }
        dto.setBelong(user.getUserNo());
        PageHelper.startPage(dto.getPageNo(), dto.getPageSize());
        List<BatteryOrder> list=orderMapper.list(dto);
        outVoGlobal.setData(list);
        return outVoGlobal;
    
    }
    

    如果没有改动的话这段代码我一定会这么写:

    public OutVoGlobal list(BatteryOrderDTO dto, UserInfo user) {

        OutVoGlobal outVoGlobal=new OutVoGlobal(EnumRetCode.SUCCESS);
        dto.setBelong(user.getUserNo());
        PageHelper.startPage(dto.getPageNo(), dto.getPageSize());
        List<BatteryOrder> list=orderMapper.list(dto);
        outVoGlobal.setData(list);
        return outVoGlobal;
    }
    

    确实是代码增加了很多行,但是我觉得这样写当我要进行断点调试的时候会很舒服。而且当别人要改我代码的时候也能一目了然。。 然后他说如果你要加上面的新需求的话可以这么写

    public OutVoGlobal list(BatteryOrderDTO dto, UserInfo user) {

        SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
        if(!StringUtils.isEmpty(dto.getStartTime())){
            try {
                sdf.parse(dto.getStartTime());
                dto.setStartTime(dto.getStartTime()+" 00:00:00");
            } catch (ParseException e) {
                dto.setStartTime("");
            }
        }
        if(!StringUtils.isEmpty(dto.getEndTime())){
            try {
                sdf.parse(dto.getEndTime());
                dto.setEndTime(dto.getEndTime()+" 23:59:59");
            } catch (ParseException e) {
                dto.setEndTime("");
            }
        }
       return new OutVoGlobal(EnumRetCode.SUCCESS).setData(orderMapper.list(dto.setBelong(user.getUserNo()))
    }
    

    我一想,这么写也可以呢。但是我还是觉得他最后那个 return 看起来太麻烦了,我又没有理由反驳他。 其实在写代码的过程中我发现他有好多的习惯我都不习惯。比如说我一般都是这么写:

    OutVoGlobal outVoGlobal=new OutVoGlobal(EnumRetCode.SUCCESS);

    …… if(StringUtils.isEmpty(XXX)){

    outVoGlobal.setCode("1000");
    outVoGlobal.setInfo(XXX+"不能为空");
    // return outVoGlobal.setCode("1000").setInfo(XXX+"不能为空");
    return outVoGlobal;
    

    } if(StringUtils.isEmpty(SSSS)){

    outVoGlobal.setCode("1000");
    outVoGlobal.setInfo(SSS+"不能为空");
    return outVoGlobal;
    

    } …… return outVoGlobal;

    如果我也用了插件的话我会这么写

    OutVoGlobal outVoGlobal=new OutVoGlobal(EnumRetCode.SUCCESS);

    …… if(StringUtils.isEmpty(XXX)){

    return outVoGlobal.setCode("1000").setInfo(XXX+"不能为空");
    

    } if(StringUtils.isEmpty(SSSS)){

     return outVoGlobal.setCode("1000").setInfo(SSS+"不能为空");
    

    } …… return outVoGlobal;

    他如果写的话会这么写:(加了 @Accessors(chain = true)的前提下)

    …… if(StringUtils.isEmpty(XXX)){

    return new OutVoGlobal().setInfo(XXX+"不能为空").setCode("1000");
    

    } if(StringUtils.isEmpty(SSSS)){

     return new OutVoGlobal().setInfo(SSS+"不能为空").setCode("1000");
    

    } …… return new OutVoGlobal(EnumRetCode.SUCCESS);

    大家觉得是先把这个变量在开始的时候声明了好还是在用到的时候直接返回好呢?
    

    然后还有别的:

    if (userData == null) return outError(outVo, EnumRetCode.NO_REGISTER, "未查询到用户信息, userNo -->{}", user.getUserNo()); else if (!userData.getPwd().equals(pwd = encrypt(user.getUserNo(), user.getPwd())))

            return outError(outVo, EnumRetCode.ERROR_PWD, "密码错误, userNo -->{} | pwdData -->{} | pwdInput -->{}", user.getUserNo(), userData.getPwd(), pwd);
    

    else if (!StringUtils.isEmpty(userData.getOpenId()) && !openid.equals(userData.getOpenId())) // 删除上一个用户信息

            redisUtil.delMapKey(param.getUserKey() + userData.getOpenId(), "userInfo", "null");
    

    这种。。。if 和 else if 他后面都跟了一行,之后 他就省去了{} 他特别喜欢这么写代码。可是我每次看都要自己看一下才知道他是怎么做的。。虽然说他只写了一行,但是我看的时候还是会脑补成我写的那样。。

    if (!"0000".equals(TokenUtil.verify(outVo, tokenMap).getCode()))

            return outVo;
    

    他还喜欢把变量声明写在一行上。。

    String openid = (String) tokenMap.get("openid"),userMapKey;

    这样的代码我找 userMapKey 就很懵逼。。

    再贴一段代码: if (userMap == null || userMap.get("userInfo") == null) {

            // 获取已绑定的用户信息
            if ((user = userInfoDao.getByOpenId(openid)) == null) return null;
    
            redisUtil.saveMapSecond(userMapKey, "userInfo", JSONObject.toJSONString(user), appParam.getCacheTime());
    
        } else
    
            user = JSONObject.parseObject(userMap.get("userInfo").toString(), UserInfo.class);
    

    反正我是看不习惯。。。大家觉得呢。这么写是好还是不好呢。。

    第 1 条附言  ·  74 天前
    讲真,我是抱着讨论的态度来发的。
    因为自己也不清楚到底哪种方法合适,因为我觉得他说的也有道理,但是我也觉得我这么写还行。
    当然了帖子绝对有我个人的主观倾向,我更愿意大家说我这么写好,所以我要在帖子里让大家更同意我的观点。
    我有问题就改。他有问题的话我就不去模仿。
    其实我主观认为两个人都有问题,我写的代码逻辑多了也臃肿,他写的让别人维护和 debug 也挺麻烦,就和之前有人回帖那样似的。两个人中和一下,可以把大的功能写在单独的方法里。
    149 回复  |  直到 2019-08-31 21:09:32 +08:00
    1  2  
        101
    ColoThor   73 天前
    我有一个疑惑,时间为什么要用字符串表示
        102
    wxl1380610   73 天前
    请加空格
        103
    diferent   73 天前   ♥ 1
    写好方法功能的注释,有对应的测试用例就行了.
    作为外部调用者难道需要关心内部的实现?
    Apache 那么多开源项目,用哪个也不需要一步步把源码看清楚啊.
        104
    CSEnter   73 天前
    想我这种初学者,看到这么精简的代码,感觉打开了一扇大门,看起来很舒服,当然也要写好注释。
        105
    yangzhezjgs   73 天前
    如果后期要改需求,这种代码改起来会爆炸
        106
    alextang95   73 天前
    链式没问题,分好行,相关注释写清楚就行。

    分过行的代码,你改需求也只需要在对应的行中增加局部变量插入修改内容。

    为了方便单步调试而制造一堆只在下一行使用的局部变量,看着就烦。尤其是变量名还起的不准确的情况下,这些局部变量名反而会影响对代码的理解,还不如一行注释。
        107
    leafre   73 天前
    @wsy190 评论你这种整天没事鸡蛋挑骨头的团队毒瘤,简直侮辱了我的流量
        108
    alextang95   73 天前
    return 过长,可能需要多增加一两个函数。
    JDK 源码挺多 return 调用函数的,不过是分成了多层调用,如果精简写起来也和你同事写的差不多。
        109
    miniwade514   73 天前
    4 个括号套在一起,莫不是得到了前苏联火箭专家的真传?
        110
    wsy190   73 天前   ♥ 3
    @leafre 整个帖子看下来就你阴阳怪气,我是昨天和人家当面讨论的,还鸡蛋里挑骨头?哪轮到你这小丑说话了?
    回你侮辱我的智商,浪费我的时间,看你回帖瞎了我的眼睛,在这找存在感来了,也不想想老子为什么怼你
        111
    hexingb   73 天前
    链式调用没问题,但是也有点过度;并且变量写一行无法接受。炫技嫌疑很重。并且不是啥大不了的技术吧,一堆破逻辑。
        112
    wr410   73 天前
    如果只是纯调用,就是应该这么精简。

    你觉得麻烦,说明是你的问题。

    因为只是纯调用,没有任何逻辑,那么你调试的地方应该放在具体的方法里,而不是这里。
        113
    wsy190   73 天前
    @DingSoung 有的时候我也想呢。是不是真的是我太菜了?(昨天晚上突然想到这事,2 点多才睡。)
    但是仔细想想我还是读的懂他的代码呢。就是读起来非常别扭。然后刚刚有人帮我指出我为什么别扭了。。


    return new OutVoGlobal(EnumRetCode.SUCCESS).setData(orderMapper.list(dto.setBelong(user.getUserNo())));
    写的问题在哪里,我们分析一下:
    执行顺序:创建 OutVoGlobal 返回值——>setData 设置数据——>orderMapper.list 执行——>dto.setBelong 封装 userNo,同时要获取 userNo
    在我们眼里的顺序是怎样的,获取 userNo ——>封装——>执行 list ——>封装返回 OutVoGlobal。可以看出它实际上需要逆着思维的顺序,user.getUserNo()这部分如果嵌套过多甚至你需要不断找。所以这种风格有点像用但是没用好的感觉。
        114
    wsy190   73 天前
    @wr410 也不算是纯调用吧,因为写的是接口,好多参数在前端判断了之后我后台也会做一下判断,这段代码之所以让我改是因为前端传过来的时间是字符串类型,之后还可能传“开始时间”的中文,我需要对他重新做一下处理。所以让我来改这段代码。
    如果按我的逻辑来讲的话 :
    先给 dto 赋值,之后再拿着 dto 去查询数据库,最后将查到的数据赋值给 OutVoGlobal。我要是进行修改的话就应该改只在第一步加验证就行了。。但是他 dto 赋值是放在了方法的最后。。流程和我的惯性思维正好是相反的。
    确实是在他的 return 之前加上我的代码就好了,但是我在没读他代码之前确实是不知道他是不是做了其他的操作,所以说我还是读了一遍他的代码。
        115
    uleh   73 天前
    哥,我觉得你应该试试新的 DateTime API
        116
    Ponze   73 天前
    嗯...想想 lambda kotlin
        117
    kkkkkrua   73 天前   ♥ 1
    上面说链式的,真的确定这符合传统链式规范))))四个括号大丈夫?

    ````java

    new OutVoGlobal(EnumRetCode.SUCCESS).setData(orderMapper.list(dto.setBelong(user.getUserNo())));

    ````


    ````java

    new OutVoGlobal(EnumRetCode.SUCCESS)
    .setBelong(user.getUserNo())
    .setData(orderMapper.list(xx));

    ````

    如果是这种我觉得没毛病
        118
    zpf124   73 天前
    Java 代码的话 国内的风格还是比较古板,比较老的. 或者应该说大企业的风格都是这样的.
    没有像国外那么多的那种财富自由的在野个人开发者, 而大企业的风格都是变化缓慢落后于当前时代的.

    而你俩恰恰各自使用了接近其中某一种的方法.

    老一点的风格 特点是没有歧义, 写的明确,但啰嗦, 比如链式都不怎么用.

    //固执的老人: 写链式的都是异端, 这样多乱 我都快看不出来到底返回的是什么了, 看我写的多清晰!
    // return new JBean().setName("xx");

    JBean b = new JBean();
    b.setName("xxx");
    b.setAge(18);
    return b;




    新一点的风格 特点是方便清晰, 写起来能少些一些废话.

    return new JBeanBuilder()
    .setName("xxx");
    .setAge(18)
    .create();


    而且好像不论哪种风格一般都不推荐把链式写一行.
        119
    guoyuchuan   73 天前
    可以尝试这样:
    return new JBeanBuilder()
    .setName("xxx");
    .setAge(18)
    .test(new XX().xx())
    .create();
    链式也是很好的一种写法,只是写到一行去了,可能可读性不好,但是不必去纠结这些;
        120
    ech0x   73 天前 via iPhone
    链式调用是链式调用的问题,我觉得链式调用挺好的简介好看。
    点号不对其是代码格式化的问题,这个需要团队统一一个代码风格,如果没有统一的代码风格你没法责怪别人。
    还有代码量越少越不容易出 Bug,也越容易找到 Bug,写好注释就行了。
        121
    ech0x   73 天前 via iPhone
    还有变量名很多你们写起来不觉得起名字很麻烦嘛……
        122
    geekc3t   73 天前
    java 代码我现在一看就头疼.那么长.看不了
        123
    hantsy   73 天前
    看完这段代码,我怀疑我写是不是 Java,太恐怖了。
        124
    xzg   73 天前
    想起来一次代码走查,用 lambda 写的代码讲了半天(雾)
        125
    coolcfan   73 天前 via Android   ♥ 1
    Debug 不要怕,IntelliJ 现在可以鼠标点选要 step in 哪个方法了
        126
    linjiayu   73 天前
    高可测才是关键
        127
    RorschachZZZ   73 天前
    可读性远比代码看着精简重要的多,而且他俩并不冲突。
        128
    zabio   73 天前
    为什么不用 kotlin 更简单
        129
    nihiue   73 天前 via Android
    写代码如说话,大的方面讲究条理分明,逻辑通顺,小的方面讲究适度冗余,简洁易懂。简短依靠的是思考以后的归纳抽象,而不是靠都写在一行,这里面的度与取舍需要写几年代码才能有所参悟
        130
    ztcaoll222   73 天前
    不建议把复杂的操作弄成一行, 但一行就能表达的不建议拆成多行
        131
    kuyuzhiqi   73 天前 via iPhone
    泄露公司代码,你可以不用来了!
        132
    tairan2006   73 天前
    其实还好…但是写一行太长了
        133
    raysmond   73 天前   ♥ 1
    ```java
    public OutVoGlobal list(BatteryOrderDTO dto, UserInfo user) {
    // 准备部分,设置查询条件
    dto.setStartTime(parseTime(dto.getStartTime()));
    dto.setEndTime(parseTime(dto.getEndTime()));
    dto.setBelong(user.getUserNo());
    PageHelper.startPage(dto.getPageNo(), dto.getPageSize());

    // 业务逻辑部分
    List<BatteryOrder> orders = orderMapper.list(dto);

    // 返回结果
    return new OutVoGlobal(EnumRetCode.SUCCESS).setData(orders);
    }
    ```

    小小的心得:

    将代码分成几个部分:准备数据、执行业务逻辑、返回
    每个部分集中放置代码,尽量简洁明了
    每个部分代码可增可减,可以增加查询条件,可以对于查询结果进行处理计算,返回结果构造和重组
        134
    biossun   73 天前
    首先通常一行不应该这么长,否则读代码的时候就像读文章中的一个很长的句子,会比较费劲。比如古龙的小说对比金庸的。

    不过写地好的话,倒也不是问题,就像金庸小说也不会真的有人明显感觉读着累。

    但这段代码有一个更为麻烦的问题,在于它有一个四层深度的嵌套调用,通常嵌套调用的代码在阅读的时候需要来回查看才能梳理清楚。而写在一行里,会加重来回查看的难度。

    所以最好能够把嵌套调用改写成链式调用(或顺序调用)。
    如果真的不好改,应该换行。
        135
    yippees   73 天前
    由右到左的压栈式设计··
    都链式出错日志跟踪?
    可复用改写

    LZ 代码正道。语法糖那种···
    糖衣炮弹···
        136
    Pastsong   73 天前
    a || b ? c : d && e 用起来
        137
    default7   73 天前
    自娱自乐~
        138
    exonuclease   73 天前 via iPhone
    这你也有意见?那你看我写的长长的一串 linq 岂不是要疯了
        139
    zongwan   73 天前
    我觉得减少了引用, 挺不错
    从接口设计上用了点心,OOP 才能写成这样
        140
    touno   73 天前
    你不觉得写代码跟说话一样吗?达到目的就好。
        141
    FrankHB   73 天前
    @yippees 语法糖?也就 ALGOL-like 用户能认为“;”不是 applicative order 的语法糖了吧?

    以行而不是 AST node 为单位处理代码的习惯也不知道是谁教的。真搞定你用的语言的“语法”了嘛?

    开火车的也了断一下好了。
    本来直觉上一开始就是 map . f x y z 的逻辑,非得
    f
    .x
    .y
    .z
    ……
    不重复.会死?
    OutVoGlobal outVoGlobal=new OutVoGlobal(EnumRetCode.SUCCESS);
    这种一个标识符近乎重复三次的还能忍着,对视觉的直觉开销和脑子里可用带宽毫无感知,降频 parse 打肿脸充胖子还装作不碍可读性的,只能呵呵……
    就自然语言这样没设计的玩意儿的用户习惯大都也懂到处重复不雅的道理,某些 PL 用户咋反而倒回去了呢?
        142
    FrankHB   73 天前
    @biossun 这就奇怪了,为什么代码必须要按“句子”来读?代码遵循所谓的语法(syntax) 又不是某些自然语言的所谓“句法”。
    照你说的,要是读古汉语这样没标点的文字,你还得非把整个段落加载进脑子里然后整段脑补出标点再分析,而不能边读边增量组句?古人真有这样的吗……现代的标点能让人省事加快分析性能,但现代人的语文本事在没标点下真该有那么差的吗?
    后面所谓嵌套写一行里加重来回看的难度就更奇怪了。对一般人来讲,难道不是因为对 cache locality 友好而更容易一次跳个来回而不是跳回来之后还得纠结在不同行里跳?我是真不懂这样的阅读直觉形成方式,是因为 cache 没 tag 导致看得足够近的东西就一团浆糊呢,还是根本就没 cache 硬记代码字符串?
        143
    FrankHB   73 天前
    想了想,大概觉察到某些分行阅读党和开火车党的理由了。(严格上来讲这里的单位也不会是正儿八斤的“行”——反正不是 py 的这种 free form 的语言要刻意多拆几行的机会多的是——而是“语句”这样的局部控制结构划分的单元。)
    理由是认为这样做更简单而具有“可读性”,这是错觉。
    深层一点的原因是误以为字面顺序可以决定操作语义的顺序。
    然而现实是这种简化规则能起作用的纯粹 linear IR 等同高级语言的状况,根本就遇不到。不管是怎么样强调所谓语句的语言都没法用它代替表达式。于是能做的无非就是先分析语句,喘口气再分析语句内的表达式而已。
    这在使可读性提高上原则上就没什么卵用,因为本来求值的相对顺序就是语义特性,依赖语法顺序在一般语言中根本不靠谱(理论上还有更麻烦的问题,stackoverflow.com/a/57619680/2307646 )。
    在性能上也是弟弟,因为原则上先语句再拆语句的分析方式会阻碍并行,跟一般人视觉的直觉上就对着干的;即便习惯了这套,基本上只能加速特定语言的特定语法的分析效率,换了个稍微不怎么熟悉的语法(直觉上没把握)立刻打回原形。
    至于所谓链式调用开火车似乎更好看,大概主要是因为这些用户用的语言的表达能力太弱,强行 map 消除语义冗余,语法上反而会更麻(罗)烦(嗦)而已……
        144
    Aresxue   73 天前
    1.除非是 lambda,否则链式调用不要超过三个(但仍然推荐无复杂业务的链式调用,只不过要找到合适的位置斩开);
    2.对于可能要复用的变量,把他的逻辑从链式调用中取出来,在链式调用中使用引用;
    3.对于可能会出错的环节,尤其像数据库相关的持久层操作一定要单独拎出来(数据库操作是程序出错最多的地方之一),一般情况下最好还要做异常捕获处理;
    4.对于可能要被重构或抽取为方法的代码片段,慎用链式调用;
        145
    GentleSadness   73 天前
    参考 @alextang95 的话,类似的话王垠也说过

    你写那么长如果是为了判断为 null,避免 exception 还好,不然希望你考虑了下需要修 BUG 的心情

    一个功能出了问题,不知道哪里有问题,我还要看那么多行,我很累的,尽量写少的,一条链路调下来会快很多
        146
    corningsun   73 天前
    你们这个复杂根本原因是方法参数和返回都没定义好

    1 方法参数应该尽可能简单,为啥要传一个 dto 作为查询参数?

    `orderMapper.list(BatteryOrderDTO dto)`

    参考一下 JPA 的写法?

    `Page<BatteryOrder> findByBelongAndOrderTimeBetween(String userNo, Date startTime, Date endTime, Pageable page)`


    2 方法的返回值为啥是 OutVoGlobal ?

    可以看一下 @RestControllerAdvice 和 @ExceptionHandler 可以简化很多代码了。
        147
    StarkWhite   73 天前
    不说别的,第一个明显是链式调用看起来更简单清晰啊,不过确实应该在 . 之前换行
        148
    daxiguaya   72 天前
    首先,如果这是公司项目的代码的话我建议还是不要动别人的好些.
    每个人每个阶段代码风格不一样很正常,如果 LZ 很在意这些会可能影响到"日常交互"的话也可以找上级要一个
    "最低限度"的标准呀,这也不是什么坏事是吧.

    回到这种链式调用的代码上来说 LZ 发的这段代码其实还算"中规中矩"把,我发段我以前写的:
    ```
    targetExamPoints.stream().filter(x -> !ObjectUtil.isEmpty(x.getParentId())).forEach(x -> x.setParentId(targetExamPointMapped.get(treeNodePrefix + sourceExamPointMapped.get(treeNodePrefix + sourceExamPointMapped.get(treeNodePrefix + x.getName()).getParentId()).getName()).getId()));
    ```
    这段代码的后果就是后来测的时候出了问题我自己都得拆开来看了.

    还有就是前排有个说 SimpleDateFormat 线程不安全的,如果 LZ 在意那段的话可以看看:
    https://stackoverflow.com/questions/41158005/can-anyone-give-me-an-example-of-showing-simpledateformat-is-thread-unsafe
        149
    koebehshian   72 天前
    golang 程序员在你们讨论风格的时候,已经被编译器纠正了
    1  2  
    关于   ·   FAQ   ·   API   ·   我们的愿景   ·   广告投放   ·   感谢   ·   实用小工具   ·   4104 人在线   最高记录 5043   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.3 · 36ms · UTC 05:43 · PVG 13:43 · LAX 21:43 · JFK 00:43
    ♥ Do have faith in what you're doing.