Как я могу улучшить читаемость и длину метода со многими операторами if?

Question

У меня есть метод с 195, если это. Вот более короткая версия:

private BigDecimal calculateTax(String country, BigDecimal amount) throws Exception {
    if(country.equals("POLAND")){
        return new BigDecimal(0.23).multiply(amount);
    }
    else if(country.equals("AUSTRIA")) {
        return new BigDecimal(0.20).multiply(amount);
    }
    else if(country.equals("CYPRUS")) {
        return new BigDecimal(0.19).multiply(amount);
    }
    else {
        throw new Exception("Country not supported");
    }
}

Я могу изменить, если это переключатели:

private BigDecimal calculateTax(String country, BigDecimal amount) throws Exception {
    switch (country) {
        case "POLAND":
            return new BigDecimal(0.23).multiply(amount);
        case "AUSTRIA":
            return new BigDecimal(0.20).multiply(amount);
        case "CYPRUS":
            return new BigDecimal(0.19).multiply(amount);
        default:
            throw new Exception("Country not supported");
    }
}

но 195 случаев все еще так долго. Как я мог улучшить читаемость и длину этого метода? Какой шаблон будет лучшим в этом случае?

Answer 1

Создайте Map<String,Double>, который сопоставляет названия стран с соответствующими налоговыми ставками:

Map<String,Double> taxRates = new HashMap<> ();
taxRates.put("POLAND",0.23);
...

Используйте это Map следующим образом:

private BigDecimal calculateTax(String country, BigDecimal amount) throws Exception {
    if (taxRates.containsKey(country)) {
        return new BigDecimal(taxRates.get(country)).multiply(amount);
    } else {
        throw new Exception("Country not supported");
    }
}

Answer 2

Поместите данные в файл XML или базу данных, а затем используйте их для заполнения словаря.Таким образом, вы можете легко изменять данные и отделять данные от логики вашего приложения.Или просто выполните запрос SQL в своем методе.

Answer 3

Не делайте этого!

Как и сейчас, ваш calculateTax метод подобен контейнеру для четырех реальных calculateTax методов,по одному для каждой из 3 стран и по одному для недействительного случая.Любой другой метод, который вы делаете в этом направлении, будет таким.Следуя этой схеме, вы получите множество переключателей (проверяющих один и тот же набор наблюдений) во многих методах, где каждый случай содержит специфику наблюдения.Но это точно полиморфизм, гораздо лучше!

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

Создайте интерфейс наподобие TaxPolicy:

interface TaxPolicy {
    BigDecimal calculateTaxFor(BigDecimal saleAmount);
}

Создайте класс, который его реализует:

class NationalSalesTaxPolicy implements TaxPolicy  {
    String countryName;
    BigDecimal salesTaxRate;

    // Insert constructor, getters, setters, etc. here

    BigDecimal calculateTaxFor(BigDecimal saleAmount) {
        return saleAmount.multiply(salesTaxRate);         
    }
}

Затем создайте объекты этого класса, по одному на страну, которую вы хотите поддерживать.Мы можем заключить этот список в новый класс NationalSalesTaxCalculator, который будет нашим универсальным средством расчета налога с продаж для любой страны:

class NationalSalesTaxCalculator {
    static Map<String, NationalSalesTaxPolicy> SUPPORTED_COUNTRIES = Stream.of(
        new NationalSalesTaxPolicy("POLAND", "0.23"),
        new NationalSalesTaxPolicy("AUSTRIA", "0.20"),
        new NationalSalesTaxPolicy("CYPRUS", "0.19")
    ).collect(Collectors.toMap(NationalSalesTaxPolicy::getCountryName, c -> c));

    BigDecimal calculateTaxFor(String countryName, BigDecimal saleAmount) {
        NationalSalesTaxPolicy country = SUPPORTED_COUNTRIES.get(countryName);
        if (country == null) throw new UnsupportedOperationException("Country not supported");

        return country.calculateTaxFor(saleAmount);
    }
}

И мы можем использовать его следующим образом:

NationalSalesTaxCalculator calculator = new NationalSalesTaxCalculator();
BigDecimal salesTax = calculator.calculateTaxFor("AUSTRIA", new BigDecimal("100"));
System.out.println(salesTax);

Некоторые ключевые преимущества, на которые следует обратить внимание:

1. Если вы добавляете новую страну, которую хотите поддерживать, вам просто нужно создать новый объект.Все методы, которые могут нуждаться в этом объекте, автоматически «делают правильные вещи», без необходимости вручную находить их все, чтобы добавить новые операторы if.
2. У вас есть возможность адаптировать функциональность по мере необходимости.Например, где я живу (Онтарио, Канада), налог с продаж не взимается за продукты.Таким образом, я мог бы создать свой собственный подкласс NationalSalesTaxPolicy, который имеет более тонкую логику.

3. Есть даже больше возможностей для улучшения.Обратите внимание, что NationalSalesTaxCalculator.calculateTaxFor() содержит некоторый код, относящийся к обработке неподдерживаемой страны.Если мы добавим новые операции к этому классу, каждому методу потребуются одинаковые проверки на нуль и выдача ошибок.

   Вместо этого это может быть реорганизовано в использование шаблона нулевого объекта .Вы реализуете UnsuppoertedTaxPolicy, который является классом, который реализует все методы интерфейса, генерируя исключения.Вот так:

   class UnsuppoertedTaxPolicy implements TaxPolicy {
       public BigDecimal calculateTaxFor(BigDecimal saleAmount) {
           throw new UnsupportedOperationException("Country not supported");
       }
   }
   

   Затем вы можете сделать

   TaxPolicy countryTaxPolicy = Optional
       .ofNullable(SUPPORTED_COUNTRIES.get(countryName))
       .orElse(UNSUPPORTED_COUNTRY);
   return countryTaxPolicy.calculateTaxFor(saleAmount);
   

   Это «централизует» все ваши исключения в одном месте, что облегчает их поиск (таким образом, легче устанавливать точки останова на), легче редактировать (в случае, если вы когда-либо захотите перенести типы исключений или изменить сообщение), и он удаляет остальную часть кода, так что ему нужно только беспокоиться о счастливом случае.

Вот рабочая демонстрация: https://repl.it/@alexandermomchilov/Polymorphism-over-ifswitch

Answer 4

В качестве каркасной задачи ...

195 дел - это не слишком долго , если , понятно, что они делают и почему, и если код внутри каждого случая минимален.Да, это долго, но это отлично читается, потому что вы точно знаете, что он делает.Длина не обязательно означает нечитаемость.

Как и другие ответы, конечно, это может быть запах кода, указывающий на то, что вы не используете OO должным образом.Но сам по себе он просто длинный, а не нечитабельный.

Answer 5

Если значения постоянны и не предназначены для регулярного изменения (в чем я сомневаюсь).Я бы представил статическую метамодель с использованием Enum:

public enum CountryList {

    AUSTRIA(BigDecimal.valueOf(0.20)),
    CYPRUS(BigDecimal.valueOf(0.19)),
    POLAND(BigDecimal.valueOf(0.23));

    private final BigDecimal countryTax;

    CountryList(BigDecimal countryTax) {
        this.countryTax = countryTax;
    }

    public BigDecimal getCountryTax() {
        return countryTax;
    }

    public static BigDecimal countryTaxOf(String countryName) {
        CountryList country = Arrays.stream(CountryList.values())
                .filter(c -> c.name().equalsIgnoreCase(countryName))
                .findAny()
                .orElseThrow(() -> new IllegalArgumentException("Country is not found in the dictionary: " + countryName));

        return country.getCountryTax();
    }
}

Затем

private BigDecimal calculateTax(String country, BigDecimal amount) throws Exception {
    return CountryList.countryTaxOf(country).multiply(amount);
}

Это читабельное, безопасное время компиляции, легко расширяемое с дополнительными метаданными для каждой страны и меньшим количеством шаблонов.

Answer 6

РЕДАКТИРОВАТЬ: пропущено @ Александр ответ;это может быть немного излишним, но он также затрагивает главное: используйте ООП.
РЕДАКТИРОВАТЬ 2: реализовано @ предложения Луана

Возможно, я что-то упустилочевидно, и это может быть немного трудно реализовать на этом позднем этапе, но это выглядит для меня как идеальный случай для объектно-ориентированного программирования :

Вы создаете класс Countryкоторый содержит все, что относится к стране, например, name и calculateTax() метод и еще много чего, и тогда ваш абонент (calculateTotalAmount() или любой другой) будет вызывать country.calculateTax(amount) вместо calculateTax(country, amount), и весь/ переключатель switch только что закончился.

Кроме того, когда вы добавляете поддержку для новой страны (скажем, где-то еще одна гражданская война, и страна распадается), вы просто добавляете все для новой страны в одном месте.вместо того, чтобы выслеживать тысячи методов с гигантскими цепями if() или switch() es ...