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

• 请不要在回答技术问题时复制粘贴 AI 生成的内容

This topic created in 2814 days ago, the information mentioned may be changed or developed.

场景保存信息时，发现手机号重复返回 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;
    }

51 replies • 2019-02-20 10:04:07 +08:00

Feb 19, 2019

不知道大佬怎么写的，要是我一般会写成连续的。
if() return;
if() return;
if() return;

whx20202

Feb 19, 2019

代码整洁之道原则：提前 return

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

如果你的多层 if else 都有代码块，那会复杂一点，但是也可以优化

yesterdaysun

Feb 19, 2019

试试 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

Feb 19, 2019 via iPhone

返回 true 要么为空要么成员唯一且和输入相等

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

else return false

danliuwo

Feb 19, 2019

switch

wutiantong

Feb 19, 2019

return CollectionUtils.isEmpty(supplierList) || supplierList.size() == BaseConstants.INT_ONE && Objects.equals(supplierList.get(BaseConstants.INT_ZERO).getId(), param.getId());

一条语句搞定，假如你搞不清||和&&的优先级关系，就自己加个括号吧。

taaaang

Feb 19, 2019

可以看看《重构改善既有代码的设计》

BingoXuan

Feb 19, 2019

连续 if 可读性更高，更容易理解。可读性和代码行数不是成反比的，有时候提高可读性，你需要更多的代码。虽然看起来很冗余，但实际上会让阅读代码的人理解起来更加清晰。

如果 python 这种缩进严格的语言，而你同事喜欢这样嵌套写。淘宝买把游标卡尺来 review 会比较好。

felixlong

Feb 19, 2019

你这 code 有 bug 啊。supplierList 为空为什么返回的是 true?

yesterdaysun

Feb 19, 2019

BaseConstants.INT_ONE 和 BaseConstants.INT_ZERO 应该就是 int 0 和 1 吧, 需要定一个常数吗?

单单为了避免魔数也不需要做到这个程度吧

MrUser

Feb 19, 2019

exists = false;
if (!exists) {}
if (!exists) {}
if (!exists) {}
return exists;

kaedea

Feb 19, 2019 via Android

scoping function 了解一下

wleexi

Feb 19, 2019

@felixlong 额不是的。为空表示新号码，可以通过检查的

qq976739120

Feb 19, 2019

典型的箭头型代码块....如何解决楼上已经说得很好了

AngryPanda

Feb 19, 2019

尽早 return

Raymon111111

Feb 19, 2019

提早 return 是正确答案.

wleexi

Feb 19, 2019

@wutiantong 想过这种，但是考虑了可读性可能不是特别高

wleexi

Feb 19, 2019

@qq976739120 不太明白

billgreen1

Feb 19, 2019

表驱动，代码大全里面提到的

Mutoo

Feb 19, 2019

BaseConstants.INT_ONE
BaseConstants.INT_ZERO

0 和 1 本身就有自带语议了，不属于魔数，没必要搞个这么长的常量吧

fuxiuyin

Feb 19, 2019

之前在哪看到一个感觉挺不错的，记得是 windows SDK 里面的风格？
// 初始化资源
int result = 0;
while(1)
{
if(xxxx)
break;
if(xxxx)
break;
// logical
result = 1;
break;
}
// 释放资源
return result;
确保一个函数一个 return，将来该起来比较容易看，也不会到处都是 return 导致忘记释放某些资源。

wutiantong

Feb 19, 2019

@wleexi 这个写法完全符合语法而且也充分表达了逻辑含义，根本算不上什么奇技淫巧，为什么可读性就不高了呢？

6diyipi

Feb 19, 2019

```
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

Feb 19, 2019

写错一个条件了，发出去的居然不能编辑了。。

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

Feb 19, 2019

@wutiantong 表达式太长了？

wutiantong

Feb 19, 2019

@wleexi 那就分行呗。

lhx2008

Feb 19, 2019

#3 @yesterdaysun 是我喜欢的风格。

hugedeffing

Feb 19, 2019

用工厂模式，或者继承来做啊，if 不就是多个条件嘛

Sapp

Feb 19, 2019

你这个反向 return 就行了，有啥优化难度

Kylinsun

Feb 19, 2019 via iPhone

用断言。

zzzzzzZ

Feb 19, 2019

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，也能增加可读性

Enivel

PRO

Feb 19, 2019

do while + break 方案比较舒服, return 太多很混乱一个函数一个出口一个入口最好

zzzzzzZ

Feb 19, 2019

总感觉你的逻辑哪里有点不对劲，可能跟参数和变量太奇葩有关，说不上来
我算了下按照你的描述漏了一个分支在最后，但是脑袋有点晕，最后少一个 if-else 你自己看看吧

JRay

Feb 19, 2019

卫语句？

ttvast

Feb 19, 2019

抛异常

yetone

Feb 19, 2019

防御式编程

nekoneko

Feb 19, 2019

BaseConstants.INT_ZERO 简直是魔鬼。。。。

vicvinc

Feb 19, 2019

最简单的

vicvinc

Feb 19, 2019

ctl+enter 直接发送了（🤦‍♂️
补充一下：

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

可以改写成：

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

shot

Feb 19, 2019

像这种判断条件不算太多的情况，写成 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

Feb 19, 2019

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

Feb 19, 2019

只是展开嵌套的话，可以写成：

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

Feb 19, 2019

是不是大概这样?
```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();
}
```