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

2019-08-30 09:36:07 +08:00
 wsy190

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

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);

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

20366 次点击
所在节点    程序员
149 条回复
FrankHB
2019-08-31 08:50:23 +08:00
@yippees 语法糖?也就 ALGOL-like 用户能认为“;”不是 applicative order 的语法糖了吧?

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

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

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

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

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

`orderMapper.list(BatteryOrderDTO dto)`

参考一下 JPA 的写法?

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


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

可以看一下 @RestControllerAdvice 和 @ExceptionHandler 可以简化很多代码了。
StarkWhite
2019-08-31 13:40:33 +08:00
不说别的,第一个明显是链式调用看起来更简单清晰啊,不过确实应该在 . 之前换行
daxiguaya
2019-08-31 14:27:31 +08:00
首先,如果这是公司项目的代码的话我建议还是不要动别人的好些.
每个人每个阶段代码风格不一样很正常,如果 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
koebehshian
2019-08-31 21:09:32 +08:00
golang 程序员在你们讨论风格的时候,已经被编译器纠正了

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://ex.noerr.eu.org/t/596413

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX