如何重构此 Java 代码段

问题描述

出于某种原因,我不禁看到了这段代码中的冗余。有人能帮我重构一下,让它更易读和更简洁吗?

if (cachedParty == null || !cachedParty.equals(partyDto)) {
    if (cachedParty == null 
        && partyDto.getSellerStatusCode()
            .equalsIgnoreCase(SellerStatusEnum.ACTIVE.getCode())) 
    {
        pricingControlList.add(convertPartyDtoToPricingControl(partyDto));
    } else if (!cachedParty.equals(partyDto) 
        && cachedParty.getSellerStatusCode()
            .equalsIgnoreCase(SellerStatusEnum.ACTIVE.getCode()) 
        && !partyDto.getSellerStatusCode()
            .equalsIgnoreCase(SellerStatusEnum.ACTIVE.getCode())
    ) {
        pricingControlList.add(convertPartyDtoToPricingControl(partyDto));
    }
                                
    partyCache.put(partyDto.getSellerServicerNumber(),partyDto);
    partiesToSaveOrUpdate.add(partyDto);
}

解决方法

  1. SellerStatusEnum.ACTIVE 的比较可以实现为接受 Supplier<String> 的函数
  2. 可以简化顶级比较以删除 cachedParty == null - 从代码中假定 partyDto 不为空,因此检查 !partyDto.equals(cachedParty) 就足够了

示例实现:

Function<Supplier<String>,Boolean> active = x -> SellerStatusEnum.ACTIVE.getCode().equalsIgnoreCase(x.get());

if (!partyDto.equals(cachedParty)) {
    boolean partyActive = active.apply(partyDto::getSellerStatusCode);
    if (null == cachedParty && partyActive 
        || null != cachedParty && !partyActive && active.apply(cachedParty::getSellerStatusCode)
    ) {
        pricingControlList.add(convertPartyDtoToPricingControl(partyDto));
    }
    partyCache.put(partyDto.getSellerServicerNumber(),partyDto);
    partiesToSaveOrUpdate.add(partyDto);
}
,

我建议制作返回 isActive(party) 的辅助方法 partyDto.getSellerStatusCode().equalsIgnoreCase(SellerStatusEnum.ACTIVE.getCode()。此外,当 cachedParty == null 时可能存在 NPE,因为 !cachedParty.equals(partyDto) 正在为 equals 调用 null。因此,考虑到 partyDto 永远不会是 null,这可以简化为 !partyDto.equals(cachedParty)

此外,您在 if(x) else if (y) 语句中调用相同的方法,因此它可以减少为一个带有 x or y 检查的 if 语句。因此,让我们重写您的声明:

if (A or B) {
    if ((A and C) or (B and D and !C)) { F() }

    G()
}

正如我们在第一段中决定的那样,A or B = B。所以表情现在看起来像

if (B) {
    if ((A and C) or (B and D and !C)) { F() }

    G()
}

// and because inside first if statement B = true,we can remove B from nested if:

if (B) {
    if ((A and C) or (D and !C)) { F() }

    G()
}

因此,通过进行这些优化,我们可以获得:

if (!partyDto.equals(cachedParty)) {

    if (cachedParty == null && isActive(partyDto) || (isActive(cachedParty) && !isActive(partyDto)) {
        pricingControlList.add(convertPartyDtoToPricingControl(partyDto));
    }

    partyCache.put(partyDto.getSellerServicerNumber(),partyDto);
    partiesToSaveOrUpdate.add(partyDto);
}

我注意到您的嵌套 if 中没有 cachedParty != null 检查,因此最终结果将如下所示:

if (!partyDto.equals(cachedParty)) {
    if (cachedParty == null && isActive(partyDto) || (cachedParty != null && isActive(cachedParty) && !isActive(partyDto)) {
        pricingControlList.add(convertPartyDtoToPricingControl(partyDto));
    }

    partyCache.put(partyDto.getSellerServicerNumber(),partyDto);
    partiesToSaveOrUpdate.add(partyDto);
}