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

2019-02-19 10:43:46 +08:00
 wleexi

场景 保存信息时,发现手机号重复返回 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;
    }
10580 次点击
所在节点    程序员
51 条回复
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
2019-02-19 11:46:33 +08:00
@wleexi 这个写法完全符合语法而且也充分表达了逻辑含义,根本算不上什么奇技淫巧,为什么可读性就不高了呢?
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
2019-02-19 11:55:22 +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;
}
wleexi
2019-02-19 11:56:52 +08:00
@wutiantong 表达式太长了?
wutiantong
2019-02-19 11:57:28 +08:00
@wleexi 那就分行呗。
lhx2008
2019-02-19 11:58:16 +08:00
#3 @yesterdaysun 是我喜欢的风格。
hugedeffing
2019-02-19 12:07:08 +08:00
用工厂模式,或者继承来做啊,if 不就是多个条件嘛
Sapp
2019-02-19 12:19:11 +08:00
你这个反向 return 就行了,有啥优化难度
Kylinsun
2019-02-19 12:40:11 +08:00
用断言。
zzzzzzZ
2019-02-19 13:56:46 +08:00
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
2019-02-19 13:57:51 +08:00
do while + break 方案比较舒服, return 太多很混乱 一个函数一个出口一个入口最好
zzzzzzZ
2019-02-19 14:05:35 +08:00
总感觉你的逻辑哪里有点不对劲,可能跟参数和变量太奇葩有关,说不上来
我算了下按照你的描述漏了一个分支在最后,但是脑袋有点晕,最后少一个 if-else 你自己看看吧
JRay
2019-02-19 14:06:26 +08:00
卫语句?
ttvast
2019-02-19 14:12:30 +08:00
抛异常
yetone
2019-02-19 14:23:14 +08:00
防御式编程
nekoneko
2019-02-19 14:33:37 +08:00
BaseConstants.INT_ZERO 简直是魔鬼。。。。
vicvinc
2019-02-19 15:08:25 +08:00
最简单的
vicvinc
2019-02-19 15:10:23 +08:00
ctl+enter 直接发送了(🤦‍♂️
补充一下:

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

可以改写成:

if (条件 1&&条件 2&&...&&条件 N)
shot
2019-02-19 15:29:15 +08:00
像这种判断条件不算太多的情况,写成 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));
}
```

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://www.v2ex.com/t/536428

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX