2015-05-03 3 views
1

Мне нужна помощь в рефакторинге этого оператора if. Я думал об объявлении процента в качестве констант. Я также думал создать метод, который включает код внутри скобок. Что еще я могу сделать?Рефакторинг if statement в java

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()); 
+0

Используйте 'else if', чтобы удалить двойные проверки и отменить вызов функции, сохраняя только проценты внутри ifs. – miniBill

+0

Ну, я сделал это сначала, но наш профессор сказал нам реорганизовать 'else if' только с' if' – flower

+0

Это не так ... Любая причина для этого? – miniBill

ответ

2

Основная проблема в вашем коде, вероятно, дублирование кода, то есть, если вы хотите изменить, например, условие, вы, вероятно, придется применить те же изменения во всех четырех условий. Таким образом, вы можете попытаться разделить общую функциональность, как вы уже предложили для условностей. Таким образом, можно определить метод

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

и обновить если-заявления соответственно:

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()); 

Теперь, есть еще дублирование в теле, если заявления. Таким образом, вы можете ввести другой метод:

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

и обновить если-заявления снова:

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); 

Теперь, если вы хотите, вы можете обнаружить образец в используемых numericals, или просто жёстко в numericals в массиве или список и использовать цикл вместо множества подобных, если-заявления:

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]); 

Это ла Этап рефакторинга полностью избавляется от дублирования, но, на мой взгляд, делает код немного менее понятным.

Edit: Обратите внимание, что я предположил, что первый условный должен быть

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

, как это на самом деле выглядит это то, что вы хотели написать. Если это не так, то первое условие должно рассматриваться отдельно.