找到你要的答案

Q:Refactoring if statement in java

Q:如果声明在java的重构

I need some help refactoring this if-statement. I thought of declaring the percentage as constants. I also thought to make a method that includes the code inside the if brackets. What else can i do?

if(totalReceiptsAmount >= getIncome() && totalReceiptsAmount <  0.20 * getIncome())
        setTaxIncrease(getBasicTax() + 0.05 * getBasicTax());
    if(totalReceiptsAmount >=  0.20 * getIncome() && totalReceiptsAmount <  0.40 * getIncome())
        setTaxIncrease(getBasicTax() - 0.05 * getBasicTax());
    if(totalReceiptsAmount >=  0.40 * getIncome() && totalReceiptsAmount <  0.60 * getIncome())
        setTaxIncrease(getBasicTax() - 0.10 * getBasicTax());
    if(totalReceiptsAmount >=  0.60 * getIncome())
        setTaxIncrease(getBasicTax() - 0.15 * getBasicTax());

我需要一些帮助重构if语句。我想声明的百分比为常量。我还想做一个方法,里面包含了括号内的代码。我还能做什么?

if(totalReceiptsAmount >= getIncome() && totalReceiptsAmount <  0.20 * getIncome())
        setTaxIncrease(getBasicTax() + 0.05 * getBasicTax());
    if(totalReceiptsAmount >=  0.20 * getIncome() && totalReceiptsAmount <  0.40 * getIncome())
        setTaxIncrease(getBasicTax() - 0.05 * getBasicTax());
    if(totalReceiptsAmount >=  0.40 * getIncome() && totalReceiptsAmount <  0.60 * getIncome())
        setTaxIncrease(getBasicTax() - 0.10 * getBasicTax());
    if(totalReceiptsAmount >=  0.60 * getIncome())
        setTaxIncrease(getBasicTax() - 0.15 * getBasicTax());
answer1: 回答1:

The main problem in your code is probably the duplication of code, meaning if you want to change e.g., the condition, you'd probably have to apply the same change in all four conditions. So you could try to factor out the common functionality, as you already suggested for the conditionals. So you could define a method

private boolean receiptsAmountIsBetweenFactorOfXAndYOfIncome(double x, double y){
    return totalReceiptsAmount >= x * getIncome() && totalReceiptsAmount < y  * getIncome();
}

and update your if-statements accordingly:

if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0, 0.2))
    setTaxIncrease(getBasicTax() + 0.05 * getBasicTax());
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.2, 0.4))
    setTaxIncrease(getBasicTax() - 0.05 * getBasicTax());
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.4, 0.6))
    setTaxIncrease(getBasicTax() - 0.10 * getBasicTax());
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.6, 1))
    setTaxIncrease(getBasicTax() - 0.15 * getBasicTax());

Now, there's still duplication in the body of the if statements. So you could introduce an other method:

private void increaseTaxByFactorOfX(double x){
    setTaxIncrease(getBasicTax() + x * getBasicTax());
}

and update the if-statements again:

if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0, 0.2))
    increaseTaxByFactorOfX(0.05);
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.2, 0.4))
    increaseTaxByFactorOfX(-0.05);
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.4, 0.6))
    increaseTaxByFactorOfX(-0.10);
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.6, 1))
    increaseTaxByFactorOfX(-0.15);

Now, if you want, you can detect a pattern in the used numericals, or simply hardcode the numericals in an array or a list and use a loop instead of multiple similar if-statements:

double[] factorOfIncome = {0, 0.2, 0.4, 0.6, 1};
double[] taxIncreaseFactor = {0.05, -0.05, -0.10, -0.15};

for(int i = 0; i<taxIncreaseFactor.length; i++)
    if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(factorOfIncome[i], factorOfIncome[i+1]))
        increaseTaxByFactorOfX(taxIncreaseFactor[i]);

This last refactoring step gets completely rid of the duplication, but in my opinion makes the code a bit less understandable.

Edit: Note that I assumed that the first conditional should be

if(totalReceiptsAmount >= 0 * getIncome() && //... 

as it really looks like this is what you intended to write. If this isn't the case, then the first conditional would need to be treated separately.

代码中的主要问题可能是代码的重复,这意味着如果你想改变,例如,条件,你可能不得不在所有四个条件下应用相同的变化。所以你可以尝试分解出公共功能,如你已经提出的条件。所以你可以定义一个方法

private boolean receiptsAmountIsBetweenFactorOfXAndYOfIncome(double x, double y){
    return totalReceiptsAmount >= x * getIncome() && totalReceiptsAmount < y  * getIncome();
}

并更新你的if语句:

if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0, 0.2))
    setTaxIncrease(getBasicTax() + 0.05 * getBasicTax());
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.2, 0.4))
    setTaxIncrease(getBasicTax() - 0.05 * getBasicTax());
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.4, 0.6))
    setTaxIncrease(getBasicTax() - 0.10 * getBasicTax());
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.6, 1))
    setTaxIncrease(getBasicTax() - 0.15 * getBasicTax());

现在,在if语句的正文中仍然有重复。所以你可以介绍另一种方法:

private void increaseTaxByFactorOfX(double x){
    setTaxIncrease(getBasicTax() + x * getBasicTax());
}

并再次更新if语句:

if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0, 0.2))
    increaseTaxByFactorOfX(0.05);
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.2, 0.4))
    increaseTaxByFactorOfX(-0.05);
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.4, 0.6))
    increaseTaxByFactorOfX(-0.10);
if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(0.6, 1))
    increaseTaxByFactorOfX(-0.15);

现在,如果你想,你可以发现在用数值模式,或简单的硬编码的数字在数组或列表并使用环代替多个类似的语句:

double[] factorOfIncome = {0, 0.2, 0.4, 0.6, 1};
double[] taxIncreaseFactor = {0.05, -0.05, -0.10, -0.15};

for(int i = 0; i<taxIncreaseFactor.length; i++)
    if(receiptsAmountIsBetweenFactorOfXAndYOfIncome(factorOfIncome[i], factorOfIncome[i+1]))
        increaseTaxByFactorOfX(taxIncreaseFactor[i]);

这最后的重构步骤完全消除了重复,但在我看来,代码变得不那么容易理解了。

编辑:注意,我认为第一个条件应该是

if(totalReceiptsAmount >= 0 * getIncome() && //... 

因为这看起来真的是你想要写的。如果不是这样的话,那么第一个条件需要单独处理。

java  if-statement  refactoring