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

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

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

18931 次点击
所在节点    程序员
149 条回复
Heylion
2019-08-30 11:26:59 +08:00
@wysnylc 他这里线程私有的,怎么有并发问题
eGlhb2Jhb2Jhbw
2019-08-30 11:28:18 +08:00
你给他的意见解决方法是“展开写”
他给你的意见解决方法是“条件语句或与当前方法主要逻辑无关的代码尽量抽离出另外一个方法”

你俩的问题不就都解决了吗?
不会因为展开写导致本方法的主要逻辑难读,也不会因为有人需要知道更详细的逻辑而难读。
no1xsyzy
2019-08-30 11:30:15 +08:00
@wysnylc 这个问题是:信息流的顺序和代码中的顺序不符,所以说了多少次,使用从左到右的书写的语言,函数调用不应该是 func(args) 而应该是 args|func,不然就必然导致分步执行容易理解,但对低级语言不友好还得栈上放东西。
但这涉及多参问题……柯里化的话这种情况反而变成 argn|(...|(arg2|(arg1|func))...) 就是倒序柯里化也不能解决括号问题如果具有主从参进行数据变换的话可能 lodash 那样的点链会更好,但 _.intersection 难受了我半天。
而且强迫症感到难受:到底求交集的时候哪个在前?
所以什么时候有图形化 dataflow graph 编程?
inhzus
2019-08-30 11:32:04 +08:00
我大概遵循的规则是:一行代码中,嵌套调用尽量不超过两层,链式调用尽量分行写。
炫技不炫技不重要,把代码写的即便没有注释也能轻松看懂就行了。
guokeke
2019-08-30 11:32:09 +08:00
什么是"可读性"?可读性有啥什么标准吗?自己看不顺眼的代码就是可读性差吗?
wysnylc
2019-08-30 11:34:34 +08:00
@Heylion #21 SimpleDateFormat 存在并发问题不建议使用甚至禁用,没有说他的代码会有问题
如果一个方法存在问题,那么在有代替品的情况下应当使用代替品
时间的格式化 java8 有 LocalDateTime java8 之前可以用 apache-dateUtils dateFormatUtils
所以 SimpleDateFormat 这种存在问题的类有必要使用?
wsy190
2019-08-30 11:35:03 +08:00
@guokeke
就是这个问题。。。
我认为写一行可读性差,他认为写很多可读性差
我也在想到底哪种方式写好呢,
ccpp132
2019-08-30 11:36:19 +08:00
第一个 case 写一行比较好。不拆开就觉得读起来困难的话可以多锻炼一下阅读代码的能力
wsy190
2019-08-30 11:39:18 +08:00
@ccpp132 也不是说读起来困难吧,只不过是觉得不能一目了然。在改的时候还要从头读。
不过这应该还是和读代码能力有关系的。
wysnylc
2019-08-30 11:39:43 +08:00
@no1xsyzy #23 比较长的链式嵌套确实存在阅读的困难,但是他这个链式写得一点都不多也不长还远远称不上"阅读困难"或者"语意不清"
比如使用 Stream.of(1,2,3).map().filter().flatMap()..... 这种大段的链式调用也有解决方案 在每一个调用后换行写注释即可
函数式编程核心就是纯粹的链式调用,其次是 builder,然而二者都是存在链式调用 所以链式调用没有任何问题
你把链式调用去了换成其他代码一样存在阅读困难,本质上不是代码怎么写,而是写代码的人的问题
loginbygoogle
2019-08-30 11:43:17 +08:00
就像一坨屎
PriestTomb
2019-08-30 11:45:10 +08:00
团队里的一个年龄较小的同事也是这种写法,链式一路到底。。
可能是我还没习惯,看他的代码真的很吃力,决心了好几次,都没看完他的代码。。
leafre
2019-08-30 11:45:30 +08:00
你看不惯,关人家什么事?你有资格评价别人的代码?
zifangsky
2019-08-30 11:47:12 +08:00
@wysnylc #14 当做局部变量可以用,没有线程安全问题。
hundan
2019-08-30 11:47:53 +08:00
功能复杂的时候 最多拆成一屏能读完的 三十行左右
KyonLi
2019-08-30 11:48:07 +08:00
拆开就会需要起很多变量名,这可是最烧脑的事情
dsg001
2019-08-30 11:48:38 +08:00
如果需要从两个人中裁掉一个,老板会认为裁掉谁比较好?

之前在类似问题下看到的回复
specita
2019-08-30 11:50:03 +08:00
我觉得两种写法都 OK 啊,可读性好不好,1、排版(链式分行) 2、链式的方法名称起得好不好
链式写得好的处理逻辑一目了然,但是还是不要链得太长了.. builder 这种除钱
MMMMMMMMMMMMMMMM
2019-08-30 11:51:18 +08:00
没问题啊,有注释就行了

没注释你让他自己改去
Jex
2019-08-30 11:51:24 +08:00
菜鸡互啄

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

https://www.v2ex.com/t/596413

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

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

© 2021 V2EX