Рефакторинг сложного если-условия

К комментарию Мартина Смита добавлю:

Помните, Карно может помочь вам упростить условие этого if.

Если у вас есть условная логика (например, что-то делать, если условие выполняется), зачем вам даже пытаться избегать их?

Лучшее решение - следовать шаблону проектирования. Шаблон проектирования на основе состояний определяет класс для каждого состояния.

Класс состояния затем инкапсулирует то, что является курсом действий для этого конкретного состояния. Это не только предотвращает появление большого количества операторов if -else.

I не проверил полностью логику, но это основная идея:

amt = adjustment.total
if (adjustment.adjustmentAccount.isIncrease
    ^ adjustment.increaseVATLine
    ^ adjustment.vatItem.isSalesType)
{
    amt = -amt;
}

entry2.setCredit(amt > 0 ? amt : 0);
entry2.setDebit(amt < 0 ? -amt : 0);

Я должен отметить, что эта логика немного отличается тем, что правильно обрабатывает отрицательное значение Adjust.total , тогда как оригинал кажется предположить (возможно, правильно), что значение всегда будет неотрицательным.

На вопрос был дан ответ, но я опубликую его здесь для тех, кому важно более чистое решение:

//Set debit if exactly one or all three are true, else set credit
if(adjustment.adjustmentAccount.isIncrease ^ adjustment.increaseVATLine ^
   adjustment.vatItem.isSalesType)
{
    entry2.setDebit(adjustment.total);
    entry2.setCredit(0d);
}
else
{
    entry2.setCredit(adjustment.total);
    entry2.setDebit(0d);
}

Похоже, у вас всего 2 случая, поэтому вы можете объединить их с помощью OR AND и т. Д.

        if (<case1expression>) {
            entry2.setCredit(adjustment.total);
            entry2.setDebit(0d);
        } else {
            entry2.setDebit(adjustment.total);
            entry2.setCredit(0d);
        }

Как это сделать ... Давайте выделим пару методов, чтобы лучше понять логику.

private void a() {
    entry2.setDebit(adjustment.total);
    entry2.setCredit(0d);
}
private void b() {
    entry2.setCredit(adjustment.total);
    entry2.setDebit(0d);
}

if (adjustment.adjustmentAccount.isIncrease) {
    if (adjustment.increaseVATLine) {
        if (adjustment.vatItem.isSalesType) {
            a();
        } else {
            b();
        }
    } else {
        if (adjustment.vatItem.isSalesType) {
            b();
        } else {
            a();
        }
    }
} else {
    if (adjustment.increaseVATLine) {
        if (adjustment.vatItem.isSalesType) {
            b();
        } else {
            a();
    }
} else {
    if (adjustment.vatItem.isSalesType) {
        a();
    } else {
        b();
    }
}

Итак, теперь, глядя на это, первый блок

if (adjustment.increaseVATLine) {
    if (adjustment.vatItem.isSalesType) {
        a();
    } else {
        b();
    }
} else {
    if (adjustment.vatItem.isSalesType) {
        b();
    } else {
        a();
    }
}

просто равносилен выполнению a () , если Adjust.increaseVATLine имеет то же значение, что и adjust.vatItem .isSalesType , b () в противном случае. Таким образом, мы можем уменьшить его:

if (adjustment.adjustmentAccount.isIncrease) {
    if (adjustment.increaseVATLine == adjustment.vatItem.isSalesType) {
        a();
    } else {
        b();
    }
} else {
    if (adjustment.increaseVATLine) {
        if (adjustment.vatItem.isSalesType) {
            b();
        } else {
            a();
        }
    } else {
        if (adjustment.vatItem.isSalesType) {
            a();
        } else {
            b();
        }
    }
}

И оставшийся блок тот же, просто поменяв местами a () и b () :

if (adjustment.adjustmentAccount.isIncrease) {
    if (adjustment.increaseVATLine == adjustment.vatItem.isSalesType) {
        a();
    } else {
        b();
    }
} else {
    if (adjustment.increaseVATLine == adjustment.vatItem.isSalesType) {
        b();
    } else {
        a();
    }
}

Итак, мы начинаем видеть логику. Если это увеличение, и увеличениеVATLine совпадает с isSalesType, то мы дебетуем, в противном случае кредит, но если это уменьшение, то мы кредитуем, только если они не совпадают. Как лучше это выразить? Ну, во-первых, назовите a () и b () умнее - теперь, когда мы можем видеть, что они делают

if (adjustment.adjustmentAccount.isIncrease) {
    if (adjustment.increaseVATLine == adjustment.vatItem.isSalesType) {
        debitEntry();
    } else {
        creditEntry();
    }
} else {
    if (adjustment.increaseVATLine == adjustment.vatItem.isSalesType) {
        creditEntry();
    } else {
        debitEntry();
    }
}

А теперь еще немного яснее. Дебитируйте счет, когда это счет увеличения и строка увеличения НДС и тип продаж, или когда это уменьшение и либо это строка уменьшения, либо это тип продаж, но не оба сразу. Эта таблица истинности помогает? Первый столбец - AdjustAmount.isIncrease ; второй - Adjust.increaseVATLine ; третий - Adjust.vatItem.isSalesType . Четвертый столбец - D для дебета, C для кредита; в скобках указано количество ИСТИННЫХ значений среди флагов.

TTT -> D (3) 
TFF -> D (1) 
TTF -> C (2)
TFT -> C (2) 
FTT -> C (2) 
FFF -> C (0)
FTF -> D (1) 
FFT -> D (1)

Теперь вы можете понять, почему работает решение @Xavier Ho; все нечетные суммы являются дебетом, четные - кредитами.

Это всего лишь один исследовательский путь; Надеюсь, это поможет.

Что вы обычно можете сделать, чтобы до некоторой степени облегчить ситуацию, так это использовать наследование.

Например, если у вас есть два класса Increase и NonIncrease , которые являются подклассами одного суперкласса, у вас может быть метод doSomething , который - ну - что-то в зависимости от того, какой у вас класс в настоящее время. Тогда вам не нужно проверять, «является ли объект do X», а просто вызовите .doSomething (), и он сделает то, для чего предназначен.

Затем вы можете пойти дальше и иметь все больше и больше подклассов для дальнейшего «уточнения» и «избегания большего количества если».

Могут быть другие возможности (в значительной степени в зависимости от вашей среды / требований), такие как указатели на функции, делегаты, шаблон стратегии (GoF) или подобные конструкции.

Вы можете использовать такую ​​таблицу истинности:

debit = ((isIncrease && increaseVATLine && !isSalesType) ||
         (isIncrease && !increaseVATLine && isSalesType) ||
         (!isIncrease && increaseVATLine && isSalesType) ||
         (!isIncrease && !increaseVATLine && !isSalesType)) ? 0 : adjustment.total;
entry2.setCredit(debit);

Здесь нет никаких «если», и вы легко можете увидеть, в каких случаях дебет равен 0. То же самое. за кредит.