applyFactors重构

Question

我继承了下面的Java函数，它以应有的方式工作，但是您必须花一分钟的时间来了解到底是怎么回事。是否有一种更简洁或优雅的方式来编码这个逻辑？

private Float applyFactors(Float originalValue, Float localFactor, Float globalFactor){
    if (globalFactor == null || globalFactor == 0){
        if (localFactor == null || localFactor == 0){
            return null;
        } else {
            return localFactor * originalValue;
        }
    } else {
        if (localFactor == null || localFactor == 0){
            return globalFactor * originalValue;
        } else {
            return localFactor * originalValue * globalFactor;
        }
    }
}

Answer 1

在这种复杂的条件情况下，我喜欢做的是预先进行比较，并给出它们的名称。这使得对代码的推理更容易，如果条件表达式很便宜，就不会有任何缺点。

然后，我继续用命名的布尔替换条件表达式，通常情况下，如何简化代码变得更加明显。我的结局是：

private Float applyFactors(Float originalValue, Float localFactor, Float globalFactor) {
    Float result = null;

    boolean hasGlobalFactor = ((globalFactor != null) && (globalFactor != 0));
    boolean hasLocalFactor =  ((localFactor != null) && (localFactor != 0));

    if (hasGlobalFactor && hasLocalFactor) {
        result = localFactor * globalFactor * originalValue;
    }
    else if (hasGlobalFactor) {
        result = globalFactor * originalValue;
    }
    else if (hasLocalFactor) {
        result = localFactor * originalValue;
    }

    return result;
}

这种方法还提高了代码的可读性。

Answer 2

我更喜欢把逻辑分解成几个部分，去掉重复的算术。我就是这样做的：

private Boolean nullOrZero(Float value) {
    return value == null || value.equals(0);
}

private Float oneIfInvalid(Float value) {
    return nullOrZero(value) ? new Float(1) : value;
}

private Float applyFactors(Float originalValue, Float localFactor, Float globalFactor) {
    if(nullOrZero(globalFactor) && nullOrZero(localFactor)) {
        return null;
    }
    return oneIfInvalid(localFactor) * oneIfInvalid(globalFactor) * originalValue;
}

有更多的方法调用，当第一个条件失败时，每个值都会重复一次nullOrZero()，但是我相信改进可读性比过早的微观优化更重要。

在这段代码中，另一件事变得明显起来，在我看来，在其他版本中，这一点并不明显；originalValue从未被检查为空或零。(这是否你的意图是另一回事。)