V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
Wsdba
V2EX  ›  Java

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

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

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

    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
    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  
       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   ·   我们的愿景   ·   实用小工具   ·   871 人在线   最高记录 6543   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 43ms · UTC 22:08 · PVG 06:08 · LAX 15:08 · JFK 18:08
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.