大家帮我看看，这代码是水平。。

[Wsdba]

刚接手的一个项目，发现这个人很喜欢这样写。

[xption]

经常遇到公司新来的同事这样写代码
习惯不好+逻辑不清晰，之前没人教过他
坐他边上手把手带他写几次就好了

[coderluan]

宝宝树，宝宝宝宝树，宝宝宝宝宝宝树

[yangzzzzzz]

给他装个 idea 按照提示优化一下代码就好了

[lrs]

这命名, 只比没有名字强一点

[lrs]

@lrs 好吧, 我看错了

[iovekkk]

由此看来，kotlin 的可空类型处理真的是太方便了

[YzSama]

看的心塞。😂

[Leviathann]

@iovekkk 然后他会写一个接受参数全是可空的方法
然后用的地方全都 double bang 而且不写注释。。

[rationa1cuzz]

像极了我之前同事写的，一个 view 6000 行

[sagaxu]

按行数算 KPI 的时候有优势

[zjsxwc]

mark, 除了命名不好外，看看大佬们会有什么写法

[Kasumi20]

服了，不会 AND OR 吗，一行代码搞定的事情，硬是拆成七八行

[kop1989]

要不提出改进方法让大家品鉴下？

[socketpeng]

@zjsxwc 我也想知道如何改进这种代码

[MrEatChicken]

想看楼主优化后的代码

[flyingyasin]

或许哪位老大哥也会这样发个帖子来嘲讽楼主写的代码

[freak118]

怎么改进啊

[DreamingCTW]

第一张图的方法...我看 if 代码块里面的返回值都是一样的.....那方法体为何不这样写.....
if ((member2 == null && member1 != null) || !member2.equals(member1)) {
return changePartPositions(member1, member2, name, org, updateTime);
}
return false;

[starsky007]

怎么改进？搜索“卫语句”。

[cstj0505]

一眼也就是缩进问题，我觉得问题不大，是正常的思维。if 判断提前返回能减少缩进，没这样做也不至于被拉出来嘲讽的地步

[liuzhaowei55]

有啥问题吗？判断逻辑清晰，没有复杂逻辑判断。
说实话代码降低心智负担才是真的重要

[zjsxwc]

@DreamingCTW
但是我反倒觉得合并条件到一个 if 中去后，反而更加烦躁，逃。。

[EggplantLover]

我觉得还好吧，能怎么精简呢，哪位大佬给个示范

[DreamingCTW]

@zjsxwc 我一眼看去挺清晰的....||左边一个条件，右边一个条件，也不算太复杂

[vanton]

if 套 if 的问题么？

这里套的也还好，不算太长。

不过最好不要这样多个 if 深层嵌套。

[DreamingCTW]

我也想看看有没有大佬来优化，因为我也经常写 if 非空判断，毕竟 Java 这个空指针异常挺恼火的

[rming]

if A and B:
return
if C:
return
return

[ww940521]

我觉得这样写挺好的逻辑一目了然，把判断条件合并起来反而看起来费劲，要去想有没有把所有的可能包含进去。一层一层判断不容易出现疏漏。我也想看看大佬优化。

[gps949]

还好吧？没看出有啥值得批判的问题。现代编译器 /解释器会对多级判断有自己的优化吧，不过是一个写 web 应用 crud 的，又不是开发编译器或操作系统，只要人读得感觉清晰，有必要非要炫技“人工优化”到一行跟 js 代码混淆一样吗？

[zhouyg]

if 套 if 其实没啥问题，跟业务逻辑对应就行

[ToDyZHu]

虽然我自己不太会写这种代码 但是我很喜欢改这种代码

[ianEros]

代码水平高应该是让应届生也能很快理解，而不是为了好看导致可读性差

[arthas2234]

if 判断逻辑简单越好，判断逻辑过长可以先把结果赋值给临时变量，变量名语义要清晰
包括里面的逻辑，也是越短越好，实在不能精简，就拆分成小的函数，方便阅读
善用 if-return：if(member != null) {...} => if(member == null) return;
变量名：flag ，a1 ，a2 之类的语义不清晰的就不要用了

[pengtdyd]

写代码的不厉害，会改别人的代码才是大佬。

[selfcreditgiving]

@starsky007 一直这么写的，这还有一个说法的啊

[starsky007]

@selfcreditgiving
一起这么写就好；《重构：改善既有代码的设计》这本书有提到这个概念，交流起来也方便些。

[sue0917]

最后他代码行数多，拿了个比你高的绩效

[SheHuannn]

这变量命名真是绝了，太直接了，像是机器人干的

[chengkai1853]

@SheHuannn 变量名并没什么问题吧？！一看函数就知道是更改成员位置信息的代码。

[Tink]

两层 if 还好吧

[naix1573]

听楼上说起来感觉优点还不少啊
逻辑清晰写起来快，一目了然看的明白，行数多了 KPI 还高 哈哈

[CharmingCheung]

图一实际就是判断两个 String 是否相等然后做不同的逻辑，直接封装个 xxUtils.equals(String a, String b)方法判断两个 String 是否相等就好了。那 changePositions 里就好阅读很多了

[weaponc]

请不要随意扔垃圾

[CharmingCheung]

图二整个过程好像都没有对对象的空值做逻辑分支，那直接一个 try-catch ，try 里 return true ，catch 里 return false 完事

[binge921]

看的心肌梗塞

[easylee]

看到这样的代码，review 根本不可能过，多次出现直接劝退......

[nicebird]

遇到这种代码不要想着怎么改，直接删了自己重新写。

[xiaofeifei8]

一群人在嘲笑曾今的自己？

[Nich0la5]

按行发工资？

[aguesuka]

语法层面还好
第一个方法, member 也许是 memberId, 第一个方法里的 name 在第二个方法里成了 positionName, 嵌套的 if 应该该改成 &&, else if 应该合并, 多个分支执行相同的代码也应该合并.
第二个方法, if 的嵌套太多了.

设计层面
dao 层查询结果是 Map
changePartyPositions 应该拆成两个函数, 一个方法不允许 null, 另一个方法只有一个 member, 同样不允许 null.
不要返回 boolean, 而是应该抛异常

另外, updateTime 是 string 类型, 而且是参数, 最坏的可能是从前端拿到的, 而且要保存到数据库, 否则有理由怀疑 changePositions 在循环体中, 同样也很糟糕

虽然不希望和他当同事, 不过这已经算 ok 的代码了, 至少看到代码我知道他想干什么.

[EscYezi]

这代码是自动生成的么
善用 optional ，合理使用 if 条件

[abobobo]

@DreamingCTW 这么写，当 member2 == null 时，就报错了，反而提高了错误几率..

[guyeu]

这么写，当任何一个参数是空的时候就不发生任何事，默默地执行结束，或者返回一个保底的`null`或者`false`，部分情况可能是符合设计意图的，很多时候其实是坑。。。

[RedBeanIce]

怎么这么多人任认为这样没问题的，提前返回就行了呀，，不用走后面那么多逻辑

[mxT52CRuqR6o5]

https://github.com/trekhleb/state-of-the-art-shitcode#-triangle-principle
这是编码原则中的 Triangle principle ，建议大家都这么写（ doge

[daimubai]

能 return 就不要 else

[LUO12826]

图二除了嵌套过多外，最里面该不会是 if(bool expr) flag = true 吧，最后该不会又返回 flag 吧

[ColdBird]

@DreamingCTW 图 1 废逻辑那么多还一堆人说没问题，能看懂才是最重要的，我就不明白用你这种简单方法难道不是更容易看懂？我不懂，但我大受震撼！

[ColdBird]

典型的逻辑堆砌能用就行，完全不想怎么简化逻辑让代码更简洁易懂，还没问题，服了。
等这种嵌套到几十层就不说易懂了

[mxT52CRuqR6o5]

https://github.com/Droogans/unmaintainable-code#nesting
这是一种避免失业的编码技巧

[GeekSuPro]

wocao, 小丑就是我自己，我也这样写

[Jooooooooo]

这写的挺好的, 最多就是提前返回可以优化一下.

[JeffersonQin]

图一有待改进，但图二真心觉得还行。。。

[cppgohan]

@mxT52CRuqR6o5 分享的这俩都挺经典, 哈哈

[weiwenhao]

@JeffersonQin
flag = true
if member1 == null {
flag = false
}

if positionsInfo == null {
flag = fase
}

类似这样做个取反逻辑会更加清晰呀

[JeffersonQin]

@weiwenhao 我感觉图里的逻辑和你给的例子不太一样，而且嵌套 if 还有好处，可以防止深层次的 null pointer exception

比方说这两天我写 dotnet 用 opencv 的 wrapper ，判空会这么写：

if (src == null)
if (src.IsDisposed)
if (src.CvPtr == IntPtr.Zero)
src.Dispose();

诚然你也可以用 goto 的写法，不过嵌套 if 在某些情况下还是更直观的

[JeffersonQin]

@JeffersonQin 打错了 if 条件都反了 直接 ctrl c/v 忘记改了

[vchroc]

@DreamingCTW Optional

[admin7785]

楼主可不可以把重构之后的代码贴出来，学习学习

[fashionash]

别的不说，dao 接口返回 JSONObject 是认真的吗？

[wangyzj]

看方法命名就感觉像是一个 void 的方法
member1 应该是 memberId
还有就是不至于写俩方法把
if 嵌套还好
不过我感觉既然 return 了，没必要 else 了

[horizon]

第二个没啥问题啊。。

[pkwenda]

尝试优化了下劝退了


[Akiya]

第一个代码应该只需要 2 行：

if ((member2 == null && member1 != null) || (member2 != null && !member2.equals(member1))) {
return ...
}
return false

第二个代码感觉没啥问题，毕竟每一步值都是上面获取的，如果后面没有其他逻辑的话其实可以把 flag=true 改成 return true

[micean]

换 kotlin

[Samuelcc]

这是典型 optional 的应用场景吧

[kerro1990]

很好，简单容易理解，没有弯弯绕绕

[raaaaaar]

改进方法就是判断的时候取反，多提前 return ，不要嵌套

[AccIdent]

return Objects.equals(mem1, mem2) ? false: changePartyPosition(...);

[honkki]

&& || 这些就硬不用呗

[ZField]

@DreamingCTW #18 单个 if 的条件不要太复杂

[linshenqi]

我喜欢 goto ，唯一出口。。

[darkcode]

请问大家在讨论什么，我怎么看不见

[zhuangzhuang1988]

正常， 能用就行

[curoky]

挺好的，写过的代码只有他能看懂，出了问题也只能他来查

[sprite82]

说没问题的，平时写的比上面怕是更不堪吧？就这么几个条件合并归纳下有这么难吗，还降低心智负担，你的心智这么难以承受，还当什么程序员？第二个，数据库查询居然拿 JsonObject 作为接收对象

[miv]

看着太难受了，这代码。
if 太多，判断太多。
好的代码应该是简洁明了的。
多思考抽象，把代码变简洁。
这样就直观了，出 bug 也会变少。
而不是这样，那么多 if ，一个月后你还看懂嘛

[miv]

JAVA 里面代码抽象有两种，一是用类抽象，二是用方法抽象。我之前就用类抽象，把 n 多 switch 都干掉了。舒服

[imycc]

if 写得太暴力，看着简单，逻辑反而弯弯绕绕地

[leokino]

@liuzhaowei55 不赞同，没有必要的行数越多越影响对代码整体的理解。

[liuzhaowei55]

@sprite82 talk is cheap show your code ，自以为是的用个卫语句，坐那里扣半天联合几个逻辑判断，后来的人谁看谁骂娘

[sprite82]

@liuzhaowei55 这里就三个条件，又不是七八个， 还有，你不写注释的吗？

[liuzhaowei55]

@leokino
业务代码中这种挺常见的，我可能就是大家所不齿的那种敲代码的，用 if 把自己想要处理的场景一层层的判断下来，看起来很烂，但一眼就能看出来想要处理的数据长什么样子。

[liuzhaowei55]

@sprite82 自己写写试试，光说不练假把式

[sprite82]

@liuzhaowei55 你当别人没写过代码呢

[sprite82]

@liuzhaowei55 18 楼已经给你写好了 自己去看

[learningman]

建议直接换 kotlin

[liuzhaowei55]

@sprite82 再认真看看 18 楼代码吧，编辑器教你怎么做人。

---

有的代码可能看起来很傻，你想说 nerver do this ，那你就给出一个更好的例子出来，我是属于那种代码能跑就行的那种人，逻辑简单清晰明了，过上一年半载谁来都能看得懂就很好了，不要让人盯着一行代码反应半天。

最后想说，公司的业务代码能做到逻辑严谨就很难得了，受限于自己技术，当时的业务要求，产品的设计能力，行业现状 == 一系列原因，才成就了现在的屌样子，有能力就自己上手改，不能就争取不要做压倒骆驼的那根稻草。

[gvliwo]

我觉得还行,就是丫的没注释,不管你自己觉得如何简单,都要加注释,因为阅读的人可能会误会
代码的话:
如果从精简的角度,确实需要优化
但是团队项目更注重的是易读性,所以并不是越精简越好.
Java 主要的目标是大型分布式,易上手.超高性能不是第一要考虑的,用一部分性能换取开发的方便才是重点,Jvm 也会优化一部分代码,至于有人说数据库用 JsonObject ,那单纯看业务需要,比如说 Redis,
曾经接手了一个烂摊子,因为纯内部环境,不需要考虑安全性,直接 js 执行 sql(甲方太恶心了,一个 sql,就算加了索引,优化了 left join ,创建了中间表等一系列手段后,依然要执行半个小时以上,我真的服了)
所以我个人认为,相比于代码的好坏, SQL 的优化好坏才能体现水平

[Wh1t3zZ]

可以看下 StringUtils.isEquals(String st1, String str2) 的实现，判断两个字符串相等并考虑到两个字符串可能为 null ，非常优雅。
return str1 == null? str2 == null : str1.equals(str2);