首页 注册 登录
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
wleexi
V2EX  ›  程序员

代码 if 嵌套过多，怎么优化比较好

  •   
  •   wleexi · 2019-02-19 10:43:46 +08:00 · 10767 次点击
    这是一个创建于 2102 天前的主题，其中的信息可能已经有所发展或是发生改变。

    场景 保存信息时，发现手机号重复返回 false 编辑时，手机号与自身之外的手机号重复，返回 false, 没有重复，或者编辑时号码不变返回 true

        private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
        List<Supplier> supplierList = this.get(shopId, param.getContactMobile());
        if (!CollectionUtils.isEmpty(supplierList)) {
            if (supplierList.size() == BaseConstants.INT_ONE) {
                Long existId = supplierList.get(BaseConstants.INT_ZERO).getId();
                if (Objects.equals(existId, param.getId())) {
                    return true;
                }
            }
            return false;
        }
        return true;
    }
  • false
  • return
  • true
  • existid
    51 条回复  •  2019-02-20 10:04:07 +08:00
    si
        1
    si  
       2019-02-19 10:47:32 +08:00   ❤️ 5
    不知道大佬怎么写的，要是我一般会写成连续的。
    if() return;
    if() return;
    if() return;
    whx20202
        2
    whx20202  
       2019-02-19 10:50:33 +08:00   ❤️ 4
    代码整洁之道原则：提前 return

    if(不满足)：
    return
    if （不满足）：
    return

    如果你的多层 if else 都有代码块，那会复杂一点，但是也可以优化
    yesterdaysun
        3
    yesterdaysun  
       2019-02-19 10:52:42 +08:00   ❤️ 1
    试试 guard clause

    ```java
    private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
    List<Supplier> supplierList = this.get(shopId, param.getContactMobile());
    if (CollectionUtils.isEmpty(supplierList)) {
    return true;
    }
    if (supplierList.size() != BaseConstants.INT_ONE) {
    return false;
    }
    Long existId = supplierList.get(BaseConstants.INT_ZERO).getId();
    return Objects.equals(existId, param.getId());
    }
    ```
    LxExExl
        4
    LxExExl  
       2019-02-19 10:53:42 +08:00 via iPhone
    返回 true 要么为空 要么成员唯一且和输入相等

    if (list is null || list.size() == 1 && list.getFirst() == input) return true

    else return false
    danliuwo
        5
    danliuwo  
       2019-02-19 10:57:21 +08:00
    switch
    wutiantong
        6
    wutiantong  
       2019-02-19 10:59:30 +08:00   ❤️ 3
    return CollectionUtils.isEmpty(supplierList) || supplierList.size() == BaseConstants.INT_ONE && Objects.equals(supplierList.get(BaseConstants.INT_ZERO).getId(), param.getId());

    一条语句搞定，假如你搞不清||和&&的优先级关系，就自己加个括号吧。
    taaaang
        7
    taaaang  
       2019-02-19 11:00:47 +08:00
    可以看看《重构 改善既有代码的设计》
    BingoXuan
        8
    BingoXuan  
       2019-02-19 11:03:02 +08:00   ❤️ 8
    连续 if 可读性更高，更容易理解。可读性和代码行数不是成反比的，有时候提高可读性，你需要更多的代码。虽然看起来很冗余，但实际上会让阅读代码的人理解起来更加清晰。

    如果 python 这种缩进严格的语言，而你同事喜欢这样嵌套写。淘宝买把游标卡尺来 review 会比较好。
    felixlong
        9
    felixlong  
       2019-02-19 11:12:27 +08:00
    你这 code 有 bug 啊。supplierList 为空为什么返回的是 true?
    yesterdaysun
        10
    yesterdaysun  
       2019-02-19 11:14:07 +08:00
    BaseConstants.INT_ONE 和 BaseConstants.INT_ZERO 应该就是 int 0 和 1 吧, 需要定一个常数吗?

    单单为了避免魔数也不需要做到这个程度吧
    MrUser
        11
    MrUser  
       2019-02-19 11:20:20 +08:00
    exists = false;
    if (!exists) {}
    if (!exists) {}
    if (!exists) {}
    return exists;
    kaedea
        12
    kaedea  
       2019-02-19 11:20:24 +08:00 via Android
    scoping function 了解一下
    wleexi
        13
    wleexi  
    OP
       2019-02-19 11:22:36 +08:00
    @felixlong 额 不是的。为空表示新号码，可以通过检查的
    qq976739120
        14
    qq976739120  
       2019-02-19 11:23:01 +08:00   ❤️ 1
    典型的箭头型代码块....如何解决楼上已经说得很好了
    AngryPanda
        15
    AngryPanda  
       2019-02-19 11:23:05 +08:00   ❤️ 1
    尽早 return
    Raymon111111
        16
    Raymon111111  
       2019-02-19 11:24:51 +08:00
    提早 return 是正确答案.
    wleexi
        17
    wleexi  
    OP
       2019-02-19 11:26:07 +08:00
    @wutiantong 想过这种，但是考虑了可读性可能不是特别高
    wleexi
        18
    wleexi  
    OP
       2019-02-19 11:27:25 +08:00
    @qq976739120 不太明白
    billgreen1
        19
    billgreen1  
       2019-02-19 11:27:52 +08:00
    表驱动，代码大全里面提到的
    Mutoo
        20
    Mutoo  
       2019-02-19 11:30:21 +08:00
    BaseConstants.INT_ONE
    BaseConstants.INT_ZERO

    0 和 1 本身就有自带语议了，不属于魔数，没必要搞个这么长的常量吧
    fuxiuyin
        21
    fuxiuyin  
       2019-02-19 11:40:39 +08:00
    之前在哪看到一个感觉挺不错的，记得是 windows SDK 里面的风格？
    // 初始化资源
    int result = 0;
    while(1)
    {
    if(xxxx)
    break;
    if(xxxx)
    break;
    // logical
    result = 1;
    break;
    }
    // 释放资源
    return result;
    确保一个函数一个 return，将来该起来比较容易看，也不会到处都是 return 导致忘记释放某些资源。
    wutiantong
        22
    wutiantong  
       2019-02-19 11:46:33 +08:00
    @wleexi 这个写法完全符合语法而且也充分表达了逻辑含义，根本算不上什么奇技淫巧，为什么可读性就不高了呢？
    6diyipi
        23
    6diyipi  
       2019-02-19 11:53:20 +08:00
    ```
    private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
    List<Supplier> supplierList = this.get(shopId, param.getContactMobile());

    if (!CollectionUtils.isEmpty(supplierList) && supplierList.size() == BaseConstants.INT_ONE) { return false; }

    if (supplierList.size() != BaseConstants.INT_ONE) { return false; }

    Long existId = supplierList.get(BaseConstants.INT_ZERO).getId();

    if (!Objects.equals(existId, param.getId())) { return false; }

    return true;
    }
    ```
    这样？
    6diyipi
        24
    6diyipi  
       2019-02-19 11:55:22 +08:00   ❤️ 1
    写错一个条件了， 发出去的居然不能编辑了。。

    private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
    List<Supplier> supplierList = this.get(shopId, param.getContactMobile());

    if (!CollectionUtils.isEmpty(supplierList) && supplierList.size() != BaseConstants.INT_ONE) { return false; }

    if (supplierList.size() != BaseConstants.INT_ONE) { return false; }

    Long existId = supplierList.get(BaseConstants.INT_ZERO).getId();

    if (!Objects.equals(existId, param.getId())) { return false; }

    return true;
    }
    wleexi
        25
    wleexi  
    OP
       2019-02-19 11:56:52 +08:00
    @wutiantong 表达式太长了？
    wutiantong
        26
    wutiantong  
       2019-02-19 11:57:28 +08:00
    @wleexi 那就分行呗。
    lhx2008
        27
    lhx2008  
       2019-02-19 11:58:16 +08:00
    #3 @yesterdaysun 是我喜欢的风格。
    hugedeffing
        28
    hugedeffing  
       2019-02-19 12:07:08 +08:00
    用工厂模式，或者继承来做啊，if 不就是多个条件嘛
    Sapp
        29
    Sapp  
       2019-02-19 12:19:11 +08:00
    你这个反向 return 就行了，有啥优化难度
    Kylinsun
        30
    Kylinsun  
       2019-02-19 12:40:11 +08:00 via iPhone
    用断言。
    zzzzzzZ
        31
    zzzzzzZ  
       2019-02-19 13:56:46 +08:00   ❤️ 1
    private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
    List<Supplier> supplierList = this.get(shopId, param.getContactMobile());
    if(supplierList == null || supplierList.size() == BaseConstants.INT_ONE){
    return false;
    }
    Long existId = supplierList.get(BaseConstants.INT_ZERO).getId();
    if (CollectionUtils.isEmpty(supplierList) && Objects.equals(existId, param.getId())) {
    return true;
    }
    }

    分支结构直接 return，某些场合可以无视嵌套前提（像行 3 和行 4 根本不相关）
    分开写是基本功，增加可读性
    如果不想写逻辑运算符也可以分开写多个 if，也能增加可读性
    LevineChen
        32
    LevineChen  
       2019-02-19 13:57:51 +08:00
    do while + break 方案比较舒服, return 太多很混乱 一个函数一个出口一个入口最好
    zzzzzzZ
        33
    zzzzzzZ  
       2019-02-19 14:05:35 +08:00
    总感觉你的逻辑哪里有点不对劲，可能跟参数和变量太奇葩有关，说不上来
    我算了下按照你的描述漏了一个分支在最后，但是脑袋有点晕，最后少一个 if-else 你自己看看吧
    JRay
        34
    JRay  
       2019-02-19 14:06:26 +08:00
    卫语句？
    ttvast
        35
    ttvast  
       2019-02-19 14:12:30 +08:00
    抛异常
    yetone
        36
    yetone  
       2019-02-19 14:23:14 +08:00
    防御式编程
    nekoneko
        37
    nekoneko  
       2019-02-19 14:33:37 +08:00
    BaseConstants.INT_ZERO 简直是魔鬼。。。。
    vicvinc
        38
    vicvinc  
       2019-02-19 15:08:25 +08:00
    最简单的
    vicvinc
        39
    vicvinc  
       2019-02-19 15:10:23 +08:00   ❤️ 1
    ctl+enter 直接发送了（🤦‍♂️
    补充一下：

    if (条件 1)
    if (条件 2)
    ...
    if(条件 N)

    可以改写成：

    if (条件 1&&条件 2&&...&&条件 N)
    shot
        40
    shot  
       2019-02-19 15:29:15 +08:00   ❤️ 1
    像这种判断条件不算太多的情况，写成 if-else 问题不大，可读性好且符合团队编码规范就行。

    如果判断条件比较多（我个人习惯是 5 个及以上），可以把每个判断逻辑写成一个 lambda 函数，再循环调用它们。
    ```
    private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
    List<Supplier> supplierList = this.get(shopId, param.getContactMobile());

    List<Predicate<List<Supplier>>> predicates = Arrays.asList(
    CollectionUtils::isEmpty,
    suppliers -> suppliers.size() == BaseConstants.INT_ONE &&
    suppliers.stream().anyMatch(it -> Objects.equals(it.getId(), param.getId()))
    // more predicates here
    );

    return predicates.stream().anyMatch(it -> it.test(supplierList));
    }
    ```
    jmk92
        41
    jmk92  
       2019-02-19 18:06:50 +08:00
    private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
    List<Supplier> supplierList = this.get(shopId, param.getContactMobile());
    if (CollectionUtils.isEmpty(supplierList)) return false;
    if (supplierList.size() != BaseConstants.INT_ONE) return false;
    Long existId = supplierList.get(BaseConstants.INT_ZERO).getId();
    if (Objects.equals(existId, param.getId())==false) return false;
    return true;
    }
    biossun
        42
    biossun  
       2019-02-19 18:32:55 +08:00
    只是展开嵌套的话，可以写成：

    private boolean checkMobileExists(Long shopId, CreateSupplierParam param) {
    List<Supplier> supplierList = this.get(shopId, param.getContactMobile());

    if (CollectionUtils.isEmpty(supplierList)) {
    return true;
    }

    if (supplierList.size() == BaseConstants.INT_ONE &&
    Objects.equals(supplierList.get(BaseConstants.INT_ZERO).getId(), param.getId())) {
    return true;
    }

    return false;
    }
    codingKingKong
        43
    codingKingKong  
       2019-02-19 18:50:41 +08:00
    是不是大概这样?
    ```java
    private boolean checkMobileExists(Long shopId, String mobile, Long id) {
    List<Long> supplierList = this.get(shopId, mobile);

    if (!CollectionUtils.isEmpty(supplierList))
    return true;
    if (supplierList.size() != 1)
    return false;
    Long existId = supplierList.get(0);
    return Objects.equals(existId, id);

    }

    private List<Long> get(long shopId, String mobile){
    return Collections.emptyList();
    }
    ```
    hv3s1
        44
    hv3s1  
       2019-02-19 18:54:19 +08:00
    if 里面疯狂套三元。 会不会被接手的人打
    ArcherD
        45
    ArcherD  
       2019-02-19 19:33:01 +08:00 via Android
    用 java 12 的 switch expression， pattern match 还要再过几个版本才能支持
    ysc3839
        46
    ysc3839  
       2019-02-19 19:39:18 +08:00 via Android   ❤️ 1
    @fuxiuyin 可以改成 do {} while (0);
    这种写法在 C 语言里面挺常见的吧？算是把 while 当 goto 用。
    kaneg
        47
    kaneg  
       2019-02-19 19:44:26 +08:00 via iPhone
    把比较饶人的判断语句写成方法，比如那个 INTONE，不去跟设计的人确认是很难理解为什么那么判断，可以用一个有意义的方法名，比如 isFirstNumber （）
    ITACHIJAMES
        48
    ITACHIJAMES  
       2019-02-19 22:24:04 +08:00   ❤️ 1
    do{
    }while(false)
    结合 break 将多级嵌套转化成同级并列
    msg7086
        49
    msg7086  
       2019-02-19 22:32:23 +08:00
    我们十年前用的 Resharper 辅助重构，可以反转 if 来简化代码，只要点几下就行了。
    lxfxf
        50
    lxfxf  
       2019-02-20 04:36:14 +08:00
    @ArcherD 那不就是 scala 嘛，逃
    guanhui07
        51
    guanhui07  
       2019-02-20 10:04:07 +08:00
    提前 return
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   1002 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 30ms · UTC 20:11 · PVG 04:11 · LAX 12:11 · JFK 15:11
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.