我有一个同事写代码特别精简。。如:
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);
反正我是看不习惯。。。大家觉得呢。这么写是好还是不好呢。。
101
ColoThor 2019-08-30 15:09:27 +08:00
我有一个疑惑,时间为什么要用字符串表示
|
102
wxl1380610 2019-08-30 15:19:42 +08:00
请加空格
|
103
diferent 2019-08-30 15:24:37 +08:00 1
写好方法功能的注释,有对应的测试用例就行了.
作为外部调用者难道需要关心内部的实现? Apache 那么多开源项目,用哪个也不需要一步步把源码看清楚啊. |
104
CSEnter 2019-08-30 15:56:15 +08:00
想我这种初学者,看到这么精简的代码,感觉打开了一扇大门,看起来很舒服,当然也要写好注释。
|
105
yangzhezjgs 2019-08-30 15:57:16 +08:00
如果后期要改需求,这种代码改起来会爆炸
|
106
alextang95 2019-08-30 16:05:29 +08:00
链式没问题,分好行,相关注释写清楚就行。
分过行的代码,你改需求也只需要在对应的行中增加局部变量插入修改内容。 为了方便单步调试而制造一堆只在下一行使用的局部变量,看着就烦。尤其是变量名还起的不准确的情况下,这些局部变量名反而会影响对代码的理解,还不如一行注释。 |
108
alextang95 2019-08-30 16:08:34 +08:00
return 过长,可能需要多增加一两个函数。
JDK 源码挺多 return 调用函数的,不过是分成了多层调用,如果精简写起来也和你同事写的差不多。 |
109
miniwade514 2019-08-30 16:10:53 +08:00
4 个括号套在一起,莫不是得到了前苏联火箭专家的真传?
|
110
wsy190 OP @leafre 整个帖子看下来就你阴阳怪气,我是昨天和人家当面讨论的,还鸡蛋里挑骨头?哪轮到你这小丑说话了?
回你侮辱我的智商,浪费我的时间,看你回帖瞎了我的眼睛,在这找存在感来了,也不想想老子为什么怼你 |
111
hexingb 2019-08-30 16:28:06 +08:00
链式调用没问题,但是也有点过度;并且变量写一行无法接受。炫技嫌疑很重。并且不是啥大不了的技术吧,一堆破逻辑。
|
112
wr410 2019-08-30 16:34:22 +08:00
如果只是纯调用,就是应该这么精简。
你觉得麻烦,说明是你的问题。 因为只是纯调用,没有任何逻辑,那么你调试的地方应该放在具体的方法里,而不是这里。 |
113
wsy190 OP @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 OP @wr410 也不算是纯调用吧,因为写的是接口,好多参数在前端判断了之后我后台也会做一下判断,这段代码之所以让我改是因为前端传过来的时间是字符串类型,之后还可能传“开始时间”的中文,我需要对他重新做一下处理。所以让我来改这段代码。
如果按我的逻辑来讲的话 : 先给 dto 赋值,之后再拿着 dto 去查询数据库,最后将查到的数据赋值给 OutVoGlobal。我要是进行修改的话就应该改只在第一步加验证就行了。。但是他 dto 赋值是放在了方法的最后。。流程和我的惯性思维正好是相反的。 确实是在他的 return 之前加上我的代码就好了,但是我在没读他代码之前确实是不知道他是不是做了其他的操作,所以说我还是读了一遍他的代码。 |
115
uleh 2019-08-30 16:46:45 +08:00
哥,我觉得你应该试试新的 DateTime API
|
116
Ponze 2019-08-30 16:48:50 +08:00
嗯...想想 lambda kotlin
|
117
kkkkkrua 2019-08-30 17:03:21 +08:00 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 2019-08-30 17:04:59 +08:00
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 2019-08-30 17:13:20 +08:00
可以尝试这样:
return new JBeanBuilder() .setName("xxx"); .setAge(18) .test(new XX().xx()) .create(); 链式也是很好的一种写法,只是写到一行去了,可能可读性不好,但是不必去纠结这些; |
120
ech0x 2019-08-30 17:19:26 +08:00 via iPhone
链式调用是链式调用的问题,我觉得链式调用挺好的简介好看。
点号不对其是代码格式化的问题,这个需要团队统一一个代码风格,如果没有统一的代码风格你没法责怪别人。 还有代码量越少越不容易出 Bug,也越容易找到 Bug,写好注释就行了。 |
121
ech0x 2019-08-30 17:22:18 +08:00 via iPhone
还有变量名很多你们写起来不觉得起名字很麻烦嘛……
|
122
geekc3t 2019-08-30 17:24:01 +08:00
java 代码我现在一看就头疼.那么长.看不了
|
123
hantsy 2019-08-30 17:24:26 +08:00
看完这段代码,我怀疑我写是不是 Java,太恐怖了。
|
124
xzg 2019-08-30 17:32:32 +08:00
想起来一次代码走查,用 lambda 写的代码讲了半天(雾)
|
125
coolcfan 2019-08-30 17:36:36 +08:00 via Android 1
Debug 不要怕,IntelliJ 现在可以鼠标点选要 step in 哪个方法了
|
126
linjiayu 2019-08-30 17:40:18 +08:00
高可测才是关键
|
127
RorschachZZZ 2019-08-30 17:41:22 +08:00
可读性远比代码看着精简重要的多,而且他俩并不冲突。
|
128
zabio 2019-08-30 18:49:45 +08:00
为什么不用 kotlin 更简单
|
129
nihiue 2019-08-30 18:53:18 +08:00 via Android
写代码如说话,大的方面讲究条理分明,逻辑通顺,小的方面讲究适度冗余,简洁易懂。简短依靠的是思考以后的归纳抽象,而不是靠都写在一行,这里面的度与取舍需要写几年代码才能有所参悟
|
130
ztcaoll222 2019-08-30 19:12:04 +08:00
不建议把复杂的操作弄成一行, 但一行就能表达的不建议拆成多行
|
131
kuyuzhiqi 2019-08-30 19:14:58 +08:00 via iPhone
泄露公司代码,你可以不用来了!
|
132
tairan2006 2019-08-30 19:29:11 +08:00
其实还好…但是写一行太长了
|
133
raysmond 2019-08-30 19:32:38 +08:00 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 2019-08-30 19:35:02 +08:00
首先通常一行不应该这么长,否则读代码的时候就像读文章中的一个很长的句子,会比较费劲。比如古龙的小说对比金庸的。
不过写地好的话,倒也不是问题,就像金庸小说也不会真的有人明显感觉读着累。 但这段代码有一个更为麻烦的问题,在于它有一个四层深度的嵌套调用,通常嵌套调用的代码在阅读的时候需要来回查看才能梳理清楚。而写在一行里,会加重来回查看的难度。 所以最好能够把嵌套调用改写成链式调用(或顺序调用)。 如果真的不好改,应该换行。 |
135
yippees 2019-08-30 21:17:32 +08:00
由右到左的压栈式设计··
都链式出错日志跟踪? 可复用改写 LZ 代码正道。语法糖那种··· 糖衣炮弹··· |
136
Pastsong 2019-08-30 21:24:51 +08:00
a || b ? c : d && e 用起来
|
137
default7 2019-08-30 21:32:35 +08:00
自娱自乐~
|
138
exonuclease 2019-08-30 23:14:31 +08:00 via iPhone
这你也有意见?那你看我写的长长的一串 linq 岂不是要疯了
|
139
zongwan 2019-08-30 23:25:13 +08:00
我觉得减少了引用, 挺不错
从接口设计上用了点心,OOP 才能写成这样 |
140
touno 2019-08-31 05:49:17 +08:00
你不觉得写代码跟说话一样吗?达到目的就好。
|
141
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 用户咋反而倒回去了呢? |
142
FrankHB 2019-08-31 08:58:17 +08:00
@biossun 这就奇怪了,为什么代码必须要按“句子”来读?代码遵循所谓的语法(syntax) 又不是某些自然语言的所谓“句法”。
照你说的,要是读古汉语这样没标点的文字,你还得非把整个段落加载进脑子里然后整段脑补出标点再分析,而不能边读边增量组句?古人真有这样的吗……现代的标点能让人省事加快分析性能,但现代人的语文本事在没标点下真该有那么差的吗? 后面所谓嵌套写一行里加重来回看的难度就更奇怪了。对一般人来讲,难道不是因为对 cache locality 友好而更容易一次跳个来回而不是跳回来之后还得纠结在不同行里跳?我是真不懂这样的阅读直觉形成方式,是因为 cache 没 tag 导致看得足够近的东西就一团浆糊呢,还是根本就没 cache 硬记代码字符串? |
143
FrankHB 2019-08-31 09:17:25 +08:00
想了想,大概觉察到某些分行阅读党和开火车党的理由了。(严格上来讲这里的单位也不会是正儿八斤的“行”——反正不是 py 的这种 free form 的语言要刻意多拆几行的机会多的是——而是“语句”这样的局部控制结构划分的单元。)
理由是认为这样做更简单而具有“可读性”,这是错觉。 深层一点的原因是误以为字面顺序可以决定操作语义的顺序。 然而现实是这种简化规则能起作用的纯粹 linear IR 等同高级语言的状况,根本就遇不到。不管是怎么样强调所谓语句的语言都没法用它代替表达式。于是能做的无非就是先分析语句,喘口气再分析语句内的表达式而已。 这在使可读性提高上原则上就没什么卵用,因为本来求值的相对顺序就是语义特性,依赖语法顺序在一般语言中根本不靠谱(理论上还有更麻烦的问题,stackoverflow.com/a/57619680/2307646 )。 在性能上也是弟弟,因为原则上先语句再拆语句的分析方式会阻碍并行,跟一般人视觉的直觉上就对着干的;即便习惯了这套,基本上只能加速特定语言的特定语法的分析效率,换了个稍微不怎么熟悉的语法(直觉上没把握)立刻打回原形。 至于所谓链式调用开火车似乎更好看,大概主要是因为这些用户用的语言的表达能力太弱,强行 map 消除语义冗余,语法上反而会更麻(罗)烦(嗦)而已…… |
144
Aresxue 2019-08-31 09:29:04 +08:00
1.除非是 lambda,否则链式调用不要超过三个(但仍然推荐无复杂业务的链式调用,只不过要找到合适的位置斩开);
2.对于可能要复用的变量,把他的逻辑从链式调用中取出来,在链式调用中使用引用; 3.对于可能会出错的环节,尤其像数据库相关的持久层操作一定要单独拎出来(数据库操作是程序出错最多的地方之一),一般情况下最好还要做异常捕获处理; 4.对于可能要被重构或抽取为方法的代码片段,慎用链式调用; |
145
GentleSadness 2019-08-31 09:51:24 +08:00
参考 @alextang95 的话,类似的话王垠也说过
你写那么长如果是为了判断为 null,避免 exception 还好,不然希望你考虑了下需要修 BUG 的心情 一个功能出了问题,不知道哪里有问题,我还要看那么多行,我很累的,尽量写少的,一条链路调下来会快很多 |
146
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 可以简化很多代码了。 |
147
StarkWhite 2019-08-31 13:40:33 +08:00
不说别的,第一个明显是链式调用看起来更简单清晰啊,不过确实应该在 . 之前换行
|
148
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 |
149
koebehshian 2019-08-31 21:09:32 +08:00
golang 程序员在你们讨论风格的时候,已经被编译器纠正了
|