大家帮我看看,这代码是水平。。 - V2EX
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
Wsdba
V2EX    Java

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

  •  
  •   Wsdba 2021-12-09 14:54:02 +08:00 20111 次点击
    这是一个创建于 1411 天前的主题,其中的信息可能已经有所发展或是发生改变。

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

    159 条回复    2022-01-01 21:47:28 +08:00
    1  2  
    xption
        1
    xption  
       2021-12-09 14:57:43 +08:00   1
    经常遇到公司新来的同事这样写代码
    习惯不好+逻辑不清晰,之前没人教过他
    坐他边上手把手带他写几次就好了
    coderluan
        2
    coderluan  
       2021-12-09 14:58:47 +08:00   9
    宝宝树,宝宝宝宝树,宝宝宝宝宝宝树
    yangzzzzzz
        3
    yangzzzzzz  
       2021-12-09 15:00:29 +08:00
    给他装个 idea 按照提示优化一下代码就好了
    lrs
        4
    lrs  
       2021-12-09 15:00:49 +08:00
    这命名, 只比没有名字强一点
    lrs
        5
    lrs  
       2021-12-09 15:02:37 +08:00
    @lrs 好吧, 我看错了
    iovekkk
        6
    a href="/member/iovekkk" class="dark">iovekkk  
       2021-12-09 15:02:50 +08:00
    由此看来,kotlin 的可空类型处理真的是太方便了
    YzSama
        7
    YzSama  
       2021-12-09 15:06:17 +08:00
    看的心塞。
    Leviathann
        8
    Leviathann  
       2021-12-09 15:06:31 +08:00 via iPhone   1
    @iovekkk 然后他会写一个接受参数全是可空的方法
    然后用的地方全都 double bang 而且不写注释。。
    rationa1cuzz
        9
    rationa1cuzz  
       2021-12-09 15:16:57 +08:00
    像极了我之前同事写的,一个 view 6000 行
    sagaxu
        10
    sagaxu  
       2021-12-09 15:19:53 +08:00 via Android   2
    按行数算 KPI 的时候有优势
    zjsxwc
        11
    zjsxwc  
       2021-12-09 15:21:38 +08:00
    mark, 除了命名不好外,看看大佬们会有什么写法
    Kasumi20
        12
    Kasumi20  
       2021-12-09 15:22:19 +08:00
    服了,不会 AND OR 吗,一行代码搞定的事情,硬是拆成七八行
    kop1989
        13
    kop1989  
       2021-12-09 15:34:40 +08:00
    要不提出改进方法让大家品鉴下?
    socketpeng
        14
    socketpeng  
       2021-12-09 15:37:56 +08:00
    @zjsxwc 我也想知道如何改进这种代码
    MrEatChicken
        15
    MrEatChicken  
       2021-12-09 15:39:37 +08:00
    想看楼主优化后的代码
    flyingyasin
        16
    flyingyasin  
       2021-12-09 15:40:06 +08:00
    或许哪位老大哥也会这样发个帖子来嘲讽楼主写的代码
    freak118
        17
    freak118  
       2021-12-09 15:40:09 +08:00
    怎么改进啊
    DreamingCTW
        18
    DreamingCTW  
       2021-12-09 15:43:48 +08:00
    第一张图的方法...我看 if 代码块里面的返回值都是一样的.....那方法体为何不这样写.....
    if ((member2 == null && member1 != null) || !member2.equals(member1)) {
    return changePartPositions(member1, member2, name, org, updateTime);
    }
    return false;
    starsky007
        19
    starsky007  
       2021-12-09 15:44:13 +08:00 via Android   11
    怎么改进?搜索“卫语句”。
    cstj0505
        20
    cstj0505  
       2021-12-09 15:46:07 +08:00
    一眼也就是缩进问题,我觉得问题不大,是正常的思维。if 判断提前返回能减少缩进,没这样做也不至于被拉出来嘲讽的地步
    liuzhaowei55
        21
    liuzhaowei55  
       2021-12-09 15:49:23 +08:00 via Android   21
    有啥问题吗?判断逻辑清晰,没有复杂逻辑判断。
    说实话代码降低心智负担才是真的重要
    zjsxwc
        22
    zjsxwc  
       2021-12-09 15:54:20 +08:00   3
    @DreamingCTW
    但是我反倒觉得合并条件到一个 if 中去后,反而更加烦躁,逃。。
    EggplantLover
        23
    EggplantLover  
       2021-12-09 15:57:20 +08:00
    我觉得还好吧,能怎么精简呢,哪位大佬给个示范
    DreamingCTW
        24
    DreamingCTW  
       2021-12-09 15:59:43 +08:00
    @zjsxwc 我一眼看去挺清晰的....||左边一个条件,右边一个条件,也不算太复杂
    vanton
        25
    vanton  
       2021-12-09 16:01:42 +08:00
    if 套 if 的问题么?

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

    不过最好不要这样多个 if 深层嵌套。
    DreamingCTW
        26
    DreamingCTW  
       2021-12-09 16:04:05 +08:00
    我也想看看有没有大佬来优化,因为我也经常写 if 非空判断,毕竟 Java 这个空指针异常挺恼火的
    rming
        27
    rming  
       2021-12-09 16:11:36 +08:00   5
    if A and B:
    return
    if C:
    return
    return
    ww940521
        28
    ww940521  
       2021-12-09 16:13:56 +08:00
    我觉得这样写挺好的逻辑一目了然,把判断条件合并起来反而看起来费劲,要去想有没有把所有的可包含进去。一层一层判断不容易出现疏漏。我也想看看大佬优化。
    gps949
        29
    gps949  
       2021-12-09 16:16:15 +08:00
    还好吧?没看出有啥值得批判的问题。现代编译器 /解释器会对多级判断有自己的优化吧,不过是一个写 web 应用 crud 的,又不是开发编译器或操作系统,只要人读得感觉清晰,有必要非要炫技“人工优化”到一行跟 js 代码混淆一样吗?
    zhouyg
        30
    zhouyg  
       2021-12-09 16:23:54 +08:00
    if 套 if 其实没啥问题,跟业务逻辑对应就行
    ToDyZHu
        31
    ToDyZHu  
       2021-12-09 16:24:11 +08:00
    虽然我自己不太会写这种代码 但是我很喜欢改这种代码
    ianEros
        32
    ianEros  
       2021-12-09 16:25:56 +08:00   1
    代码水平高应该是让应届生也能很快理解,而不是为了好看导致可读性差
    arthas2234
        33
    arthas2234  
       2021-12-09 16:26:06 +08:00
    if 判断逻辑简单越好,判断逻辑过长可以先把结果赋值给临时变量,变量名语义要清晰
    包括里面的逻辑,也是越短越好,实在不能精简,就拆分成小的函数,方便阅读
    善用 if-return:if(member != null) {...} => if(member == null) return;
    变量名:flag ,a1 ,a2 之类的语义不清晰的就不要用了
    pengtdyd
        34
    pengtdyd  
       2021-12-09 16:26:10 +08:00
    写代码的不厉害,会改别人的代码才是大佬。
    selfcreditgiving
        35
    selfcreditgiving  
       2021-12-09 16:32:11 +08:00
    @starsky007 一直这么写的,这还有一个说法的啊
    starsky007
        36
    starsky007  
       2021-12-09 16:34:26 +08:00 via Android
    @selfcreditgiving
    一起这么写就好;《重构:改善既有代码的设计》这本书有提到这个概念,交流起来也方便些。
    sue0917
        37
    sue0917  
       2021-12-09 16:37:37 +08:00 via Android
    最后他代码行数多,拿了个比你高的绩效
    SheHuannn
        38
    SheHuannn  
       2021-12-09 16:40:42 +08:00
    这变量命名真是绝了,太直接了,像是机器人干的
    chengkai1853
        39
    chengkai1853  
       2021-12-09 16:51:49 +08:00
    @SheHuannn 变量名并没什么问题吧?!一看函数就知道是更改成员位置信息的代码。
    Tink
        40
    Tink  
    PRO
       2021-12-09 16:52:16 +08:00 via Android
    两层 if 还好吧
    naix1573
        41
    naix1573  
       2021-12-09 16:53:34 +08:00
    听楼上说起来感觉优点还不少啊
    逻辑清晰写起来快,一目了然看的明白,行数多了 KPI 还高 哈哈
    CharmingCheung
        42
    CharmingCheung  
       2021-12-09 16:54:02 +08:00   1
    图一实际就是判断两个 String 是否相等然后做不同的逻辑,直接封装个 xxUtils.equals(String a, String b)方法判断两个 String 是否相等就好了。那 changePositions 里就好阅读很多了
    weaponc
        43
    weaponc  
       2021-12-09 17:12:50 +08:00   5
    请不要随意扔垃圾
    CharmingCheung
        44
    CharmingCheung  
       2021-12-09 17:15:20 +08:00
    图二整个过程好像都没有对对象的空值做逻辑分支,那直接一个 try-catch ,try 里 return true ,catch 里 return false 完事
    binge921
        45
    binge921  
       2021-12-09 17:18:09 +08:00
    看的心肌梗塞
    easylee
        46
    easylee  
       2021-12-09 17:21:21 +08:00
    看到这样的代码,review 根本不可能过,多次出现直接劝退......
    nicebird
        47
    nicebird  
       2021-12-09 17:24:33 +08:00
    遇到这种代码不要想着怎么改,直接删了自己重新写。
    xiaofeifei8
        48
    xiaofeifei8  
       2021-12-09 17:30:56 +08:00   2
    一群人在嘲笑曾今的自己?
    Nich0la5
        49
    Nich0la5  
       2021-12-09 17:31:52 +08:00
    按行发工资?
    aguesuka
        50
    aguesuka  
       2021-12-09 17:47:01 +08:00
    语法层面还好
    第一个方法, member 也许是 memberId, 第一个方法里的 name 在第二个方法里成了 positionName, 嵌套的 if 应该该改成 &&, else if 应该合并, 多个分支执行相同的代码也应该合并.
    第二个方法, if 的嵌套太多了.

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

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

    虽然不希望和他当同事, 不过这已经算 ok 的代码了, 至少看到代码我知道他想干什么.
    EscYezi
        51
    EscYezi  
       2021-12-09 17:53:03 +08:00 via iPhone
    这代码是自动生成的么
    善用 optional ,合理使用 if 条件
    abobobo
        52
    abobobo  
       2021-12-09 18:02:46 +08:00
    @DreamingCTW 这么写,当 member2 == null 时,就报错了,反而提高了错误几率..
    guyeu
        53
    guyeu  
       2021-12-09 18:08:10 +08:00
    这么写,当任何一个参数是空的时候就不发生任何事,默默地执行结束,或者返回一个保底的`null`或者`false`,部分情况可能是符合设计意图的,很多时候其实是坑。。。
    RedBeanIce
        54
    RedBeanIce  
       2021-12-09 18:27:06 +08:00 via iPhone
    怎么这么多人任认为这样没问题的,提前返回就行了呀,,不用走后面那么多逻辑
    mxT52CRuqR6o5
        55
    mxT52CRuqR6o5  
       2021-12-09 18:33:12 +08:00   6
    https://github.com/trekhleb/state-of-the-art-shitcode#-triangle-principle
    这是编码原则中的 Triangle principle ,建议大家都这么写( doge
    daimubai
        56
    daimubai  
       2021-12-09 18:37:36 +08:00
    能 return 就不要 else
    LUO12826
        57
    LUO12826  
       2021-12-09 18:37:49 +08:00
    图二除了嵌套过多外,最里面该不会是 if(bool expr) flag = true 吧,最后该不会又返回 flag 吧
    ColdBird
        58
    ColdBird  
       2021-12-09 18:40:03 +08:00
    @DreamingCTW 图 1 废逻辑那么多还一堆人说没问题,能看懂才是最重要的,我就不明白用你这种简单方法难道不是更容易看懂?我不懂,但我大受震撼!
    ColdBird
        59
    ColdBird  
       2021-12-09 18:41:49 +08:00
    典型的逻辑堆砌能用就行,完全不想怎么简化逻辑让代码更简洁易懂,还没问题,服了。
    等这种嵌套到几十层就不说易懂了
    mxT52CRuqR6o5
        60
    mxT52CRuqR6o5  
       2021-12-09 18:45:03 +08:00   3
    https://github.com/Droogans/unmaintainable-code#nesting
    这是一种避免失业的编码技巧
    GeekSuPro
        61
    GeekSuPro  
       2021-12-09 18:47:08 +08:00
    wocao, 小丑就是我自己,我也这样写
    Jooooooooo
        62
    Jooooooooo  
       2021-12-09 18:55:19 +08:00
    这写的挺好的, 最多就是提前返回可以优化一下.
    JeffersonQin
        63
    JeffersonQin  
       2021-12-09 18:56:11 +08:00
    图一有待改进,但图二真心觉得还行。。。
    cppgohan
        64
    cppgohan  
       2021-12-09 19:02:56 +08:00
    @mxT52CRuqR6o5 分享的这俩都挺经典, 哈哈
    weiwenhao
        65
    weiwenhao  
       2021-12-09 19:04:12 +08:00
    @JeffersonQin
    flag = true
    if member1 == null {
    flag = false
    }

    if positiOnsInfo== null {
    flag = fase
    }

    类似这样做个取反逻辑会更加清晰呀
    JeffersonQin
        66
    JeffersonQin  
       2021-12-09 19:13:55 +08:00
    @weiwenhao 我感觉图里的逻辑和你给的例子不太一样,而且嵌套 if 还有好处,可以防止深层次的 null pointer exception

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

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

    诚然你也可以用 goto 的写法,不过嵌套 if 在某些情况下还是更直观的
    JeffersonQin
        67
    JeffersonQin  
       2021-12-09 19:15:09 +08:00
    @JeffersonQin 打错了 if 条件都反了 直接 ctrl c/v 忘记改了
    vchroc
        68
    vchroc  
       2021-12-09 19:18:53 +08:00
    @DreamingCTW Optional
    admin7785
        69
    admin7785  
       2021-12-09 19:23:54 +08:00 via iPhone
    楼主可不可以把重构之后的代码贴出来,学习学习
    fashionash
        70
    fashionash  
       2021-12-09 19:33:28 +08:00
    别的不说,dao 接口返回 JSONObject 是认真的吗?
    wangyzj
        71
    wangyzj  
       2021-12-09 19:43:08 +08:00
    看方法命名就感觉像是一个 void 的方法
    member1 应该是 memberId
    还有就是不至于写俩方法把
    if 嵌套还好
    不过我感觉既然 return 了,没必要 else 了
    horizon
        72
    horizon  
       2021-12-09 19:56:03 +08:00
    第二个没啥问题啊。。
    pkwenda
        73
    pkwenda  
       2021-12-09 20:20:54 +08:00
    尝试优化了下劝退了

    ![carbon.png]( https://s2.loli.net/2021/12/09/3yxPRG8sIXnverA.png)
    Akiya
        74
    Akiya  
       2021-12-09 20:27:22 +08:00
    第一个代码应该只需要 2 行:

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

    第二个代码感觉没啥问题,毕竟每一步值都是上面获取的,如果后面没有其他逻辑的话其实可以把 flag=true 改成 return true
    micean
        75
    micean  
       2021-12-09 20:29:10 +08:00
    换 kotlin
    Samuelcc
        76
    Samuelcc  
       2021-12-09 20:31:08 +08:00 via Android
    这是典型 optional 的应用场景吧
    kerro1990
        77
    kerro1990  
       2021-12-09 20:46:02 +08:00
    很好,简单容易理解,没有弯弯绕绕
    raaaaaar
        78
    raaaaaar  
       2021-12-09 21:11:47 +08:00 via Android
    改进方法就是判断的时候取反,多提前 return ,不要嵌套
    AccIdent
        79
    AccIdent  
       2021-12-09 21:13:58 +08:00   2
    return Objects.equals(mem1, mem2) ? false: changePartyPosition(...);
    honkki
        80
    honkki  
       2021-12-09 21:26:11 +08:00
    && || 这些就硬不用呗
    ZField
        81
    ZField  
       2021-12-09 21:36:56 +08:00
    @DreamingCTW #18 单个 if 的条件不要太复杂
    linshenqi
        82
    linshenqi  
       2021-12-09 21:43:05 +08:00
    我喜欢 goto ,唯一出口。。
    darkcode
        83
    darkcode  
       2021-12-09 21:49:58 +08:00
    请问大家在讨论什么,我怎么看不见
    zhuangzhuang1988
        84
    zhuangzhuang1988  
       2021-12-09 22:07:43 +08:00
    正常, 能用就行
    curoky
        85
    curoky  
       2021-12-09 22:37:57 +08:00 via Android
    挺好的,写过的代码只有他能看懂,出了问题也只能他来查
    sprite82
        86
    sprite82  
       2021-12-09 23:11:02 +08:00   1
    说没问题的,平时写的比上面怕是更不堪吧?就这么几个条件合并归纳下有这么难吗,还降低心智负担,你的心智这么难以承受,还当什么程序员?第二个,数据库查询居然拿 JsonObject 作为接收对象
    miv
        87
    miv  
       2021-12-09 23:30:24 +08:00 via Android
    看着太难受了,这代码。
    if 太多,判断太多。
    好的代码应该是简洁明了的。
    多思考抽象,把代码变简洁。
    这样就直观了,出 bug 也会变少。
    而不是这样,那么多 if ,一个月后你还看懂嘛
    miv
        88
    miv  
       2021-12-09 23:33:18 +08:00 via Android
    JAVA 里面代码抽象有两种,一是用类抽象,二是用方法抽象。我之前就用类抽象,把 n 多 switch 都干掉了。舒服
    imycc
        89
    imycc  
       2021-12-09 23:36:56 +08:00
    if 写得太暴力,看着简单,逻辑反而弯弯绕绕地
    leokino
        90
    leokino  
       2021-12-09 23:41:50 +08:00
    @liuzhaowei55 不赞同,没有必要的行数越多越影响对代码整体的理解。
    liuzhaowei55
        91
    liuzhaowei55  
       2021-12-09 23:45:40 +08:00 via Android
    @sprite82 talk is cheap show your code ,自以为是的用个卫语句,坐那里扣半天联合几个逻辑判断,后来的人谁看谁骂娘
    sprite82
        92
    sprite82  
       2021-12-09 23:49:48 +08:00
    @liuzhaowei55 这里就三个条件,又不是七八个, 还有,你不写注释的吗?
    liuzhaowei55
        93
    liuzhaowei55  
       2021-12-09 23:52:59 +08:00 via Android
    @leokino
    业务代码中这种挺常见的,我可能就是大家所不齿的那种敲代码的,用 if 把自己想要处理的场景一层层的判断下来,看起来很烂,但一眼就能看出来想要处理的数据长什么样子。
    liuzhaowei55
        94
    liuzhaowei55  
       2021-12-09 23:53:43 +08:00 via Android
    @sprite82 自己写写试试,光说不练假把式
    sprite82
        95
    sprite82  
       2021-12-10 00:02:54 +08:00
    @liuzhaowei55 你当别人没写过代码呢
    sprite82
        96
    sprite82  
       2021-12-10 00:03:56 +08:00
    @liuzhaowei55 18 楼已经给你写好了 自己去看
    learningman
        97
    learningman  
       2021-12-10 01:04:02 +08:00
    建议直接换 kotlin
    liuzhaowei55
        98
    liuzhaowei55  
       2021-12-10 01:14:51 +08:00
    @sprite82 再认真看看 18 楼代码吧,编辑器教你怎么做人。

    ---

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

    最后想说,公司的业务代码能做到逻辑严谨就很难得了,受限于自己技术,当时的业务要求,产品的设计能力,行业现状 == 一系列原因,才成就了现在的样子,有能力就自己上手改,不能就争取不要做压倒骆驼的那根稻草。
    Gav1nw
        99
    Gav1nw  
       2021-12-10 01:46:59 +08:00
    我觉得还行,就是丫的没注释,不管你自己觉得如何简单,都要加注释,因为阅读的人可能会误会
    代码的话:
    如果从精简的角度,确实需要优化
    但是团队项目更注重的是易读性,所以并不是越精简越好.
    Java 主要的目标是大型分布式,易上手.超高性能不是第一要考虑的,用一部分性能换取开发的方便才是重点,Jvm 也会优化一部分代码,至于有人说数据库用 JsonObject ,那单纯看业务需要,比如说 Redis,
    曾经接手了一个烂摊子,因为纯内部环境,不需要考虑安全性,直接 js 执行 sql(甲方太恶心了,一个 sql,就算加了索引,优化了 left join ,创建了中间表等一系列手段后,依然要执行半个小时以上,我真的服了)
    所以我个人认为,相比于代码的好坏, SQL 的优化好坏才能体现水平
    Wh1t3zZ
        100
    Wh1t3zZ  
       2021-12-10 01:55:58 +08:00   3
    可以看下 StringUtils.isEquals(String st1, String str2) 的实现,判断两个字符串相等并考虑到两个字符串可能为 null ,非常优雅。
    return str1 == null? str2 == null : str1.equals(str2);
    1  2  
    关于     帮助文档     自助推广系统     博客     API     FAQ     Solana     1121 人在线   最高记录 6679       Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 62ms UTC 18:06 PVG 02:06 LAX 11:06 JFK 14:06
    Do have faith in what you're doing.
    ubao msn snddm index pchome yahoo rakuten mypaper meadowduck bidyahoo youbao zxmzxm asda bnvcg cvbfg dfscv mmhjk xxddc yybgb zznbn ccubao uaitu acv GXCV ET GDG YH FG BCVB FJFH CBRE CBC GDG ET54 WRWR RWER WREW WRWER RWER SDG EW SF DSFSF fbbs ubao fhd dfg ewr dg df ewwr ewwr et ruyut utut dfg fgd gdfgt etg dfgt dfgd ert4 gd fgg wr 235 wer3 we vsdf sdf gdf ert xcv sdf rwer hfd dfg cvb rwf afb dfh jgh bmn lgh rty gfds cxv xcv xcs vdas fdf fgd cv sdf tert sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf shasha9178 shasha9178 shasha9178 shasha9178 shasha9178 liflif2 liflif2 liflif2 liflif2 liflif2 liblib3 liblib3 liblib3 liblib3 liblib3 zhazha444 zhazha444 zhazha444 zhazha444 zhazha444 dende5 dende denden denden2 denden21 fenfen9 fenf619 fen619 fenfe9 fe619 sdf sdf sdf sdf sdf zhazh90 zhazh0 zhaa50 zha90 zh590 zho zhoz zhozh zhozho zhozho2 lislis lls95 lili95 lils5 liss9 sdf0ty987 sdft876 sdft9876 sdf09876 sd0t9876 sdf0ty98 sdf0976 sdf0ty986 sdf0ty96 sdf0t76 sdf0876 df0ty98 sf0t876 sd0ty76 sdy76 sdf76 sdf0t76 sdf0ty9 sdf0ty98 sdf0ty987 sdf0ty98 sdf6676 sdf876 sd876 sd876 sdf6 sdf6 sdf9876 sdf0t sdf06 sdf0ty9776 sdf0ty9776 sdf0ty76 sdf8876 sdf0t sd6 sdf06 s688876 sd688 sdf86