Java:重构多个if语句

Question

"Multiple if statement"是标准的代码异味。有很多方法可以重构它。在简单的情况下，我尝试使用策略模式。但是最近我运行了使用带有 int 范围的多个 if 语句的代码。而且我不知道如何使这段代码干净。

下面是此类代码的示例：

public void calculate(int i) {
    if(i > 0 && i < 5) {
        // do smth
    } else if(i > 4 && i < 10) {
        //do smth
    } else if (i >= 10 && i <20 ) {
        //do smth
    }
    //...
    else if (i > 90 && i < 100) {
        //do smth
    }
}

我试图将每个范围提取到某个逻辑单元以使用策略，但所有这些 if 语句都只是从此方法中移出并且根本不会消失。

有没有办法重构这样的 if 语句（即检查 int 范围的位置）？

Answer 1

这不是说明这一点的最佳示例。

可以说这段代码相对干净。 它当然很容易阅读，也许比复杂的策略模式更易于阅读。 当多态性发挥作用时，我会想到策略。

此示例可以使用 Map 轻松清理，其中键是范围内的最大值，该值是 JDK 8 中 java.util.function 包的接口类型引用。 也许IntToDoubleFunction是你需要的。

为什么您的计算方法似乎什么也没做？ 它不应该返回计算结果吗？

private Map<Integer, IntToDoubleFunction> strategy = new TreeMap<Integer, IntToDoubleFunction>() {{
    put(5, new IntToDoubleFunction() { // add function here });
    put(90, new IntToDoubleFunction() { // add function here });
}};
void calculate(int input) {
    double result = 0.0;
    for (Integer maxValueInRange: this.strategy.keySet()) {
        if (input <= maxValueInRange) {
            result = this.strategy.get(maxValueInRange).applyAsDouble(input);
            break;
            // what happens to result?
        }
    }
}

Answer 2

我认为

没有 if 分支就没有好方法可以做到这一点，但您可以跳过下限，因为您正在使用else if

例如

public void calculate(int i) {
    if( i <= 0) {
        return;
    } else if(i < 5) {
        // do smth
    } else if(i < 10) {
        //do smth
    } else if (i <20 ) {
        //do smth
    }
    //...
    else if (i < 100) {
        //do smth
    }
}

编辑：更新了它以包括 0 案例。谢谢 Stultuske

Answer 3

尝试更改

if(i > 0 && i < 5) {
    // do smth
} else if(i > 4 && i < 10) {
    //do smth
} else if (i >= 10 && i <20 ) {
    //do smth
}
//...
else if (i > 90 && i < 100) {
    //do smth
}

像这样：

if(i > 0){
  if ( i < 5 ){
  } else if (i < 10 ) {
  //
  }
}

更简单，并导致相同的结果

Answer 4

要考虑的可能设计模式：

面对类似的问题（范围测试ICAO Squawk值的稀疏性），我也对代码相当混乱的外观感到困惑。

我的解决方案是一个执行范围测试的静态函数，然后简单地调用它来计算"之间"测试（如果函数之间有 Java，我不知道...... 以下是我如何解决这个问题的片段;对于所有事情来说都不是一个好的模式，但也许这激发了另一种解决方案的想法。

/**
 * Range test shorthand 
 * 
 * @param value - value to test
 * @param min   - lower bound
 * @param max   - upper bound
 * 
 * @return  true | false  
 */
private static boolean inRange(int value, int min, int max){
    return min <= value && value <= max;
}

这是我使用它的方式：

        // not a pre-defined code...  run some range tests next to quess
    if (inRange(squawk,41,57)) {
        message = "test";
    }
    else if (inRange(squawk,100,400)){
        message = "Unique Purpose and Experimental activities";  // ud 17-OCT
    }
    else if (inRange(squawk,100,700)){  // Note! this is an overlap defined IN Order 7110.66E
        message = "Oceanic Airspace";   // ud 17-OCT
    }
    else if (inRange(squawk,1207,1272)){
        message = "DVFR Aircraft (USA)";  // ud 17-OCT
    }
    else if (inRange(squawk,1273,1275)){
        message = "Calibration Performance Monitoring Equipment";
    }

[...]

使用简单的级联对我来说不起作用，因为它是一个稀疏的集合，并且还涉及一张地图...... （这是其中的一部分，供参考）：

private static final Map<Integer,String> codes;
static {
    codes = new HashMap<Integer,String>();
    /* unremarkable codes */
    codes.put(0,    "");
    codes.put(0000, "");  // there message defined for 0000 in 7110.66E spec, but I'm using an empty string
    codes.put(0021, "VFR below 5000ft.");
    codes.put(0022, "VFR above 5000ft.");
    codes.put(0033, "Parachute Drop Operations");
    codes.put(0100, "Airport Flight Operations");
    codes.put(0500, "External ARTCC subsets");
    codes.put(0600, "External ARTCC subsets");
    codes.put(0700, "External ARTCC subsets");