Чем плохо отсутствие сокрытия реализации

public void updateAmountValues(List<TransactionResultItem> transactionResultItems) {
        for (TransactionResultItem transaction : transactionResultItems) {
            Accountaccount= getAccountById(transaction.getAccountId());
            if ((transaction.getMainTransaction()
                    && ! transaction.getTransactionTypeId().equals(4)
                    && ! transaction.getTransactionTypeId().equals(5)
                    && ! transaction.getTransactionTypeId().equals(6))
                    ||
                    (! transaction.getMainTransaction() &&
                            (transaction.getTransactionTypeId().equals(5) &&
                                    ((account.getAccountTypeId().equals(AccountType.INCOME_TYPE_ID) ||
                                            account.getAccountTypeId().equals(AccountType.OTHER_INCOME_TYPE_ID)) &&
                                            transaction.getAmount() > 0)
                                    || (transaction.getAccountId().equals(getSalesTaxPayableAccountId()) && transaction.getAmount() > 0)
                                    || ((account.getAccountTypeId().equals(AccountType.EXPENSE_TYPE_ID) ||
                                    account.getAccountTypeId().equals(AccountType.OTHER_EXPENSE_TYPE_ID)) && transaction.getAmount() < 0))
                            || (transaction.getTransactionTypeId().equals(1) &&
                            (account.getAccountTypeId().equals(AccountType.INCOME_TYPE_ID) ||
                                    account.getAccountTypeId().equals(AccountType.OTHER_INCOME_TYPE_ID)) &&
                            transaction.getAmount() < 0)
                            || (transaction.getTransactionTypeId().equals(2) &&
                            (account.getAccountTypeId().equals(AccountType.INCOME_TYPE_ID) ||
                                    account.getAccountTypeId().equals(AccountType.OTHER_INCOME_TYPE_ID)) &&
                            transaction.getAmount() > 0)
                    )) {
                Doubleamount= transaction.getAmount();
                transaction.setAmount(-amount);
            }
        }
    }

Что не так с этим кодом

Автор классов TransactionResultItem и Account заставляет их пользователей (возможно, себя же) писать нечитаемые простыни кода, копаясь во всех самых потаённых деталях их реализации, да ещё и с применением магических литералов, вместо того, чтобы позволить пользователям задавать экземплярам классов несколько простых вопросов:

транзакция принадлежит к такому-то виду транзакций?
транзакция является входящей?
транзакция является исходящей?
транзакция является верхнеуровневой или вложенной?
с транзакции следует заплатить налог?

и наконец:

знак суммы транзакции должен быть скорректирован?

и выполнять над ними пару простых действий:

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

Отрефакторенная версия кода

После анализа и серии выделений методов исходный код приобрёл следующий вид:

publicvoidcorrectAmountSign(List<TransactionResultItem> transactionResultItems) {
        for (TransactionResultItem transaction : transactionResultItems) {
            transaction.correctAmountSign();
        }
    }

class TransactionResultItem {
    // ...
    public void function correctAmountSign() {
            if (this.isIncorrectAmountSign()) {
                this.negateAmount();
            }
    }
    public boolean isIncorrectAmountSign() {
        Account account = getAccountById(this.getAccountId());
        return this.isSubTransaction() && this.isSuperDuper()
                    ||
                    this.isMainTransaction() && (
                            account.isIncome()     && this.isNittyGritty()     && this.getAmount() > 0
                         || account.isTaxPayable()                             && this.getAmount() > 0
                         || account.isExpense()                                && this.getAmount() < 0
                         || account.isIncome()     && this.isHocusPocus()      && this.getAmount() < 0
                         || account.isIncome()     && this.isJeepersCreepers() && this.getAmount() > 0
                        );
    }
    public void function negateAmount() {
        this.amount = - this.amount;
    }
    public boolean isSuperDuper() {
        return ! this.getTransactionTypeId().equals(4)
            && ! this.isNittyGritty()
            && ! this.getTransactionTypeId().equals(6);
    }
    public boolean isNittyGritty() {
        return this.getTransactionTypeId().equals(5);
    }
    public boolean isHocusPocus() {
        return this.getTransactionTypeId().equals(1);
    }
    public boolean isJeepersCreepers() {
        return this.getTransactionTypeId().equals(2);
    }
    public boolean isSubTransaction() {
        return this.getMainTransaction();
    }
    public boolean isMainTransaction() {
        return !this.getMainTransaction();
    }
    // ...
}

class Account {
    // ...
    public boolean isIncome() {
        return this.getAccountTypeId().equals(AccountType.INCOME_TYPE_ID) ||
                                    this.getAccountTypeId().equals(AccountType.OTHER_INCOME_TYPE_ID);
    }
    public boolean function isExpense() {
        return this.getAccountTypeId().equals(AccountType.EXPENSE_TYPE_ID) ||
                                    this.getAccountTypeId().equals(AccountType.OTHER_EXPENSE_TYPE_ID)
    }
    public boolean function isTaxPayable() {
        return this.id.equals(getSalesTaxPayableAccountId());
    }

    // ...
}

Ещё мне понадобилось одно переименование и форматирование условия.

Это было первое приближение — дальше нужно «шарить» в предметной области, чтобы сделать код ещё более читаемым.

Главное здесь то, что запись условия в виде кода стала сильно ближе к тому, как человек рассказал бы словами об этом условии.

Читая обновлённый код, разработчику уже не нужно будет мысленно «переводить» выражение transaction.getMainTransaction() как «это вспомогательная транзакция» в силу того, что у данной транзакции есть main-транзакция. И далее в том же духе. А после перевода переведённое значение ещё нужно удерживать в фокусе внимания до конца прочтения всего условия. Что называется «пока дочитал — начало забыл».

P.S. Единственное, что меня в этом условии беспокоит — это его первая часть. Вторая часть вся зависит от знака суммы, а первая не зависит:

this.isSubTransaction() && this.isSuperDuper()

Теория

Длина функции
Название функции
Ребусы в коде
Выделение функции

Литература

Мартин Р. Чистый код: создание, анализ и рефакторинг. Библиотека программиста. — СПб.: Питер, 2017.
Фаулер М. Рефакторинг: улучшение существующего кода. — СПб: Символ-Плюс, 2016
FunctionLength