Интерфейс с только получателями - это запах кода?

(Я видел этот вопрос , но первый ответ касается автоматических свойств больше, чем дизайна, а второй говорит , что нужно скрыть код хранилища данных от потребителя , что я не уверен, что я хочу / мой код делает, так что хотелось бы услышать другое мнение)

У меня есть две очень похожие сущности, HolidayDiscountи они RentalDiscountпредставляют скидки на длину как «если это длится, по крайней мере numberOfDays, percentскидка применима». Таблицы имеют fks для различных родительских объектов и используются в разных местах, но там, где они используются, существует общая логика для получения максимальной применимой скидки. Например, у a HolidayOfferесть число HolidayDiscounts, и при расчете его стоимости нам нужно определить соответствующую скидку. То же самое для аренды и RentalDiscounts.

Поскольку логика та же, я хочу сохранить ее в одном месте. Вот что делают следующий метод, предикат и компаратор:

Optional<LengthDiscount> getMaxApplicableLengthDiscount(List<LengthDiscount> discounts, int daysToStay) {
    if (discounts.isEmpty()) {
        return Optional.empty();
    }
    return discounts.stream()
            .filter(new DiscountIsApplicablePredicate(daysToStay))
            .max(new DiscountMinDaysComparator());
}

public class DiscountIsApplicablePredicate implements Predicate<LengthDiscount> {

    private final long daysToStay;

    public DiscountIsApplicablePredicate(long daysToStay) {
        this.daysToStay = daysToStay;
    }

    @Override
    public boolean test(LengthDiscount discount) {
        return daysToStay >= discount.getNumberOfDays();
    }
}

public class DiscountMinDaysComparator implements Comparator<LengthDiscount> {

    @Override
    public int compare(LengthDiscount d1, LengthDiscount d2) {
        return d1.getNumberOfDays().compareTo(d2.getNumberOfDays());
    }
}

Поскольку единственной необходимой информацией является количество дней, я получаю интерфейс

public interface LengthDiscount {

    Integer getNumberOfDays();
}

И две сущности

@Entity
@Table(name = "holidayDiscounts")
@Setter
public class HolidayDiscount implements LengthDiscount {

    private BigInteger percent;

    private Integer numberOfDays;

    public BigInteger getPercent() {
        return percent;
    }

    @Override
    public Integer getNumberOfDays() {
        return numberOfDays;
    }

}

@Entity
@Table(name = "rentalDiscounts")
@Setter
public class RentalDiscount implements LengthDiscount {

    private BigInteger percent;

    private Integer numberOfDays;

    public BigInteger getPercent() {
        return percent;
    }

    @Override
    public Integer getNumberOfDays() {
        return numberOfDays;
    }
}

Интерфейс имеет один метод получения, который реализуют две сущности, который, конечно, работает, но я сомневаюсь, что это хороший дизайн. Он не представляет никакого поведения, учитывая, что значение не является свойством. Это довольно простой случай, у меня есть еще пара подобных, более сложных случаев (с 3-4 получателями).

Мой вопрос, это плохой дизайн? Какой подход лучше?

Я бы сказал, что ваш дизайн немного ошибочен.

Одним из возможных решений было бы создание интерфейса под названием IDiscountCalculator.

decimal CalculateDiscount();

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

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

Чтобы ответить на ваш вопрос: вы не можете сделать вывод о запахе кода, если в интерфейсе есть только геттеры.

Примером нечеткой метрики для запахов кода являются раздутые интерфейсы со многими методами (не с геттерами или без). Они имеют тенденцию нарушать принцип сегрегации интерфейса.

На семантическом уровне принцип единой ответственности также не должен нарушаться. Я склонен нарушаться, если в контракте на интерфейс вы видите несколько разных проблем, которые не относятся к одной теме. Иногда это трудно определить, и вам нужен некоторый опыт, чтобы определить это.

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

Единственное, что меня беспокоит в связи с вашим подходом, это то, что вы работаете с OR-Mappings. В этом маленьком случае это не проблема. Технически это нормально. Но я не буду оперировать объектами OR-Mapped. У них может быть слишком много разных состояний, которые вы наверняка не учитываете при выполнении операций над ними ...

OR-Mapped объекты могут быть (предположительно JPA) временными, постоянными отсоединенными без изменений, постоянными прикрепленными без изменений, постоянными отсоединенными измененными, постоянными присоединенными измененными ... если вы используете проверку объектов на этих объектах, вы также не сможете увидеть, был ли объект проверен или нет. В конце концов вы можете идентифицировать более 10 различных состояний, которые может иметь объект OR-Mapped. Все ли ваши операции обрабатывают эти состояния правильно?

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