刚接手的一个项目，发现这个人很喜欢这样写。
This topic created in 1650 days ago, the information mentioned may be changed or developed.
 |
101
fml87 Dec 10, 2021
楼里居然有相当部分的人认为这种是好代码，这种代码习惯一两周就能堆出大几千行的屎山
|
 |
102
xuanbg Dec 10, 2021  1
if (member1 == null || member1.equals(member2)){
return false; } changePartyPosition(...); |
|
103
xuanbg Dec 10, 2021
@DreamingCTW 你可能看错了，数据验证逻辑应该是：member1 不能为空，且 member1 不能与 member2 相同。
|
 |
104
ryd994 Dec 10, 2021 via Android
1 应该使用 &&
2 可以使用 fail fast pattern ，也可以使用&& 怎么说呢，可读性不好，但是实际性能没啥区别，毕竟编译器啥都能给你优化掉 |
 |
105
snw Dec 10, 2021 via Android
@xuanbg
如果 number1==null 且 number2!=null ，原代码是会执行 ChangePartyPosition 的，和你的代码不同。 |
 |
106
hackk Dec 10, 2021
请问只有我看不到 LZ 发的代码吗？
|
|
107
xuanbg Dec 10, 2021
@snw 嗯嗯，这样可以简化为：
return Object.equals(member1, member2) ? false : changePartyPosition(...); |
|
108
snw Dec 10, 2021 via Android
如果业务逻辑预计一年以上不会变，那么可以尝试优化。如果预计一年以内会变化，那么楼主图中这种代码其实挺好的。
你看上面回帖的代码，至少已经有 2 位写 bug 了🐶 |
|
109
xuanbg Dec 10, 2021
不过换我来写代码的话，判断 member1 是否和 member2 相同这个逻辑肯定是写在 changePartyPosition 这个方法里面的。
|
 |
110
justest123 Dec 10, 2021
这个帖子从昨天看到今天，有个有趣的现象，有贴自己“精简”代码的，好像不是被人说逻辑写错了，就是被从其他方面挑出“问题”
逻辑易懂跟代码精简貌似不是那么好兼得（狗头（逃 |
 |
111
justfindu Dec 10, 2021
为什么会觉得一定要把代码合并缩进, 缩写 if 才是高端写法, 这样写法后来接手人不是能够一眼就看出逻辑吗. 这样写法整体会浪费资源吗?
|
|
113
xuanbg Dec 10, 2021
@justest123 不是，恰恰是因为楼主贴的代码表达的逻辑晦涩难明，才造成大家的理解错误。经过严密推导，逻辑其实简单到令人发指：就是判断 member1 是否和 member2 相同，相同就返回 false ，不同就执行 changePartyPosition 方法。为啥要写这么复杂，我猜是因为需要预防空抛出指针异常而已。
|
 |
114
chenshun00 Dec 10, 2021 1
难道没有人想到把这几个参数聚合成一个对象，然后这些判断放到对象里边去操作么。就算后边要加参数，要改变对象行为不是很 easy
|
 |
115
BlueCropland Dec 10, 2021
哈哈，仿佛看了以前的自己， 堆 shi 山
|
|
116
BlueCropland Dec 10, 2021
6666
|
 |
117
pengjl Dec 10, 2021
看到了以前的自己
|
 |
118
steptodream Dec 10, 2021
这个问题是普遍存在的，本质就是很难在别人眼里不是傻 x ，最典型场景开车
|
 |
119
1109599636 Dec 10, 2021
接手重构项目的时候碰见这种代码难道不开心吗， 逻辑顺着代码就能看懂， 要是把判断逻辑合并到一行光看懂什么意思就得花好长时间，重构的时候万一没考虑全了还容易出 bug 。。。
|
 |
120
hzw94 Dec 10, 2021
不写注释通通捶死！
拿到无效数据(null)就应该直接 return 结束代码，简介明了。不该 if..else 判空再执行。 |
 |
121
chihiro2014 Dec 10, 2021
这代码写得好，让人有工作量
|
 |
122
Cloutain Dec 10, 2021
可读性还行 ，反正编译器会合并条件
|
 |
123
chenyu0532 Dec 10, 2021
没有注释，没有 log
除了这两个我不觉得有什么不妥。 可改进，但是个人感觉意义不大 接手的人不用费脑子一看就能明白 |
 |
124
hejw19970413 Dec 10, 2021
第一点 ： 两个函数高度相似能不能合并成一个
第二点 ： 可以先行判断的前提条件可以提到前面部分 第三点 ： 可以直接返回的判断是不是可以不用嵌套 我感觉简单改可以按这个改 。 |
 |
125
sky101001 Dec 10, 2021
又不是看不懂
要求项目代码的每一行都经得起推敲才是强人所难，屎山是在所难免的，这种算不上好代码，但好歹一眼就能看懂，没必要特地拎出来说 |
 |
126
hakr Dec 10, 2021
@starsky007 #19 是的, 我也喜欢这样写, 先逆向判断, 先处理逻辑少的条件
|
 |
127
SS0001 Dec 10, 2021
也不用过度考虑简化，像楼上说的，简化也要看值不值嘛。
非要说优化的话 图 1 已经很多朋友说了，图二的话我个人认为改用 try catch 也可以。 private boolean changePartyPositions(String member1, String member2, String position...) { boolean flag = false; try { JSONObject member1Json = partyMemberDao.getInfoById(member1); JSONObject memberPositionsInfo = memberPositionsDao.getInfo(member1, p...); flag = memberPositionsDao.delete(member1, positionName, org, updateT...) } catch (NullPointerException e) { // do nothing. } return flag; } |
 |
128
shawnsh Dec 10, 2021 via Android
如果没有事情干，可以换种写法
|
 |
129
ganbuliao Dec 10, 2021
//个人喜欢的逻辑 觉得更符合阅读习惯
if (member2 == null && member1 != null) { return changepartypositlons(); } if (member2 != null && !member2.equals(member1)) { return changepartypositlons(); } return false; |
|
130
hakr Dec 10, 2021
|
 |
131
yl666 Dec 10, 2021
这种代码风格是把自己需要用到的场景写出来了，剩下的全是一个场景，有些考虑不到的场景就很容易走到 else 里面，造成 bug ，我更喜欢把所有场景列出来的方式来写
|
 |
132
anonydmer Dec 10, 2021
其实楼主第二章图没贴全，导致大家优化不彻底
|
|
133
hakr Dec 10, 2021
|
 |
134
leokino Dec 10, 2021
@liuzhaowei55 你没理解我的意思，绝大部分情况下这种条件判断粗略看过就可以了，本身就已经非常清楚了，类似这样的代码过多，说明你要多阅读数倍的代码量，才能理解整体代码的意思，实际上大大降低了可读性。
|
 |
135
Quarter Dec 10, 2021 via Android
我觉得挺好的啊，条理清晰的，方便阅读
|
 |
136
crayygy Dec 10, 2021
append 一个代码版本的？ 图片似乎丢失了(还是我这里 load 不出来...
|
 |
137
comoyi Dec 10, 2021
结构清晰，按最细粒度划分分支，对未来的变更留了空间，之后迭代只要加 else 代码不需要改 if 条件代码
|
 |
139
tqrj Dec 10, 2021
下面居然有评论觉得这样写挺好的 /。。。。+
|
 |
140
28Sv0ngQfIE7Yloe Dec 10, 2021
咱先不说条件判断
这个 member 1 、member2 大家都能忍受吗？ |
 |
141
fkdog Dec 10, 2021
认为这种写法好的显然是不写单元测试的那种。三个嵌套 if 你就能搞出 2^3=8 种分支，你有精力去写这么多的 test-case 吗？
能写出这样代码的一般都是逻辑很混乱的那种，不会去整理思考分支结构的前因后果，然后 debug 的时候发现空指针或者报错，然后顺势往里插一个 if 来解决问题。。 |
 |
142
ytmsdy Dec 10, 2021
降低预期！降低预期！降低预期！
现在 360 行，行行转码农，进来的人水平参差不齐！ 只要是能跑，没 bug ，看得懂的代码，都行。别给我出 bug ，别让我半夜起来修生产环境都行！ 否正这种代码看多了会脑溢血，你不接受能怎么办捏？帮他改？教他写？那花的不还是你自己的时间么。 有这时间，多刷几部剧不好么。 |
|
143
chenshun00 Dec 10, 2021
```java
public class CulContext { public String member1; public String member2; public String name; public String org; public String updateTime; public boolean canChangePartyPositions() { if (changeCondition()) { return true; } return !Objects.equals(member1, member2); } private boolean changeCondition() { return member2 == null && member1 != null; } } ``` === ```java public class BizService { public boolean changePositions(CulContext culContext) { if (culContext.canChangePartyPositions()) { return changePartyPosition(culContext); } return false; } private boolean changePartyPosition(CulContext culContext) { //提前返回降低代码复杂度 //biz return false; } } ``` |
 |
145
litchinn Dec 10, 2021
先说我的观点：尽管代码有优化空间，比如判断条件可以合并，但是其实并不严重，至少这么多楼下来大家看懂这个代码的占多数不是吗？
其次就是楼上说时间久了变成屎山，其实任何代码时间久了不重构都会变成屎山，拿这个举例，如果参数 member 再多几个，判断条件的组合就会爆炸（排列组合 Cmn ），这种时候使用只要还是这样 if else 写再怎么合并条件、提前返回还是看不懂的，这种情况我能想到的解决办法就是使用 **规则引擎** 这种模式去做，将每个判断条件的处理变成一个 handler 。 |
 |
146
cominbril Dec 10, 2021 1
@starsky007 正解，看评论怎么感觉大家都有点不入行啊
|
 |
147
lolizeppelin Dec 10, 2021
逻辑确实有问题
这里逻辑确实可以简化 mb1 != mb2 && mb1!=null return true return false |
 |
148
siweipancc Dec 10, 2021 via iPhone
学完重构与代码简洁之道，写出来的代码同事看不懂。（你这代码这么多个 return ，else if 也不嵌套？）年后准备跑路
|
 |
149
statumer Dec 10, 2021 via iPhone
个人觉得大多数情况，大家眼中只有好深莫测的优美代码和浅显易懂的烂代码。只要能避免 bad case 就不是烂代码。
|
|
150
snw Dec 11, 2021 via Android
|
 |
151
akira Dec 11, 2021
代码不够优雅是水平问题
各种特例有考虑到说明已经用心了 |
 |
152
Lonenso Dec 11, 2021 via iPhone
封装一个 member 类，把 org 和 name 作为属性，然后实现 swap
|
 |
153
whi147 Dec 11, 2021 via iPhone
提早 return 就行了
|
 |
154
gyh1996 Dec 11, 2021
```java
if (member1 == null ? member2 == null : !member1.equals(member2)) { return false; } return changePartyPositions(member1, member2, name, org, updateTime); ``` |
 |
155
bzl132 Dec 11, 2021
写代码的应该是新手，总体感觉有以下两个问题：
1.方法的设计就有问题，为什么要拆成两个方法，明明就是干一个事情的，入参还都一样 2.一般方法应该就两个部分，一是什么情况下什么也不做直接返回，二是具体要干什么，这些都没考虑清楚，直接根据业务条件来写，后面等业务变化后一定是一坨屎 |
 |
156
tohuer00 Dec 11, 2021
第一个乍一看不觉得有问题，仔细看发现只是为了 equals 避免空指针写了这么一大堆 if ，改成 Objects.equals 就好。
如果是业务条件需要的类似判断场景，那么像图一这么写两层 if 嵌套其实比楼上一些人的全写到一个 if 括号里更容易阅读。 |
 |
158
wangshanshan Dec 17, 2021
第二个 感觉 最好不要多层 if 嵌套，这个还好层数不多 不是很复杂。 建议可以反着写 if(a==null) if(b==null)
|
Select Language