Ist eine Schnittstelle mit nur Gettern ein Codegeruch?

(Ich habe diese Frage gesehen , aber die erste Antwort bezieht sich mehr auf automatische Eigenschaften als auf Design, und die zweite lautet: Verstecke den Datenspeichercode vor dem Verbraucher , was ich nicht sicher bin, was ich will / mein Code tut. also würde ich gerne eine andere Meinung hören)

Ich habe zwei sehr ähnliche Einheiten, HolidayDiscountund RentalDiscount, die Länge Rabatte als repräsentieren ‚ wenn es mindestens dauert numberOfDays, eine percentErmäßigung gilt‘. Die Tabellen haben fks für verschiedene übergeordnete Entitäten und werden an verschiedenen Orten verwendet. Wo sie jedoch verwendet werden, gibt es eine gemeinsame Logik, um den maximal anwendbaren Rabatt zu erhalten. Zum Beispiel HolidayOfferhat a eine Reihe von HolidayDiscounts, und bei der Berechnung seiner Kosten müssen wir den anwendbaren Rabatt herausfinden. Gleiches gilt für Vermietung und RentalDiscounts.

Da die Logik dieselbe ist, möchte ich sie an einem einzigen Ort aufbewahren. Das tun die folgende Methode, das folgende Prädikat und der folgende Komparator:

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

Da nur die Anzahl der Tage benötigt wird, erhalte ich eine Schnittstelle als

public interface LengthDiscount {

    Integer getNumberOfDays();
}

Und die beiden Einheiten

@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;
    }
}

Die Schnittstelle hat eine Single-Getter-Methode, die die beiden Entitäten implementieren, was natürlich funktioniert, aber ich bezweifle, dass es ein gutes Design ist. Es stellt kein Verhalten dar, da das Halten eines Werts keine Eigenschaft ist. Dies ist ein ziemlich einfacher Fall, ich habe noch ein paar ähnliche, komplexere Fälle (mit 3-4 Gettern).

Meine Frage ist, ist das ein schlechtes Design? Was ist ein besserer Ansatz?

Ich würde sagen, Ihr Design ist ein wenig fehlgeleitet.

Eine mögliche Lösung wäre die Erstellung einer Schnittstelle namens IDiscountCalculator.

decimal CalculateDiscount();

Jetzt würde jede Klasse, die einen Rabatt gewähren muss, diese Schnittstelle implementieren. Wenn der Rabatt die Anzahl der Tage verwendet, ist es der Benutzeroberfläche eigentlich egal, da dies die Implementierungsdetails sind.

Die Anzahl der Tage gehört wahrscheinlich zu einer abstrakten Basisklasse, wenn alle Rabattklassen dieses Datenelement benötigen. Wenn es klassenspezifisch ist, deklarieren Sie einfach eine Eigenschaft, auf die je nach Bedarf öffentlich oder privat zugegriffen werden kann.

Um Ihre Frage zu beantworten: Sie können keinen Codegeruch abschließen, wenn eine Schnittstelle nur Getter enthält.

Ein Beispiel für eine Fuzzy-Metrik für Code-Gerüche sind aufgeblähte Schnittstellen mit vielen Methoden (nicht, ob Getter oder nicht). Sie neigen dazu, das Prinzip der Grenzflächentrennung zu verletzen.

Auf semantischer Ebene sollte auch das Prinzip der Einzelverantwortung nicht verletzt werden. Ich neige dazu, verletzt zu werden, wenn im Schnittstellenvertrag mehrere verschiedene Probleme behandelt werden, die nicht ein Thema sind. Dies ist manchmal schwer zu identifizieren und Sie benötigen einige Erfahrung, um es zu identifizieren.

Auf der anderen Seite sollten Sie wissen, ob Ihre Schnittstelle Setter definiert. Das liegt daran, dass Setter wahrscheinlich ihren Status ändern, das ist ihre Aufgabe. Die hier zu stellende Frage: Macht das Objekt hinter der Schnittstelle einen Übergang von einem gültigen Zustand in einen anderen gültigen Zustand? Dies ist jedoch nicht hauptsächlich ein Problem der Schnittstelle, sondern die Implementierung des Schnittstellenvertrags des implementierenden Objekts.

Das einzige Problem, das ich mit Ihrem Ansatz habe, ist, dass Sie mit OP-Mappings arbeiten. In diesem kleinen Fall ist dies kein Problem. Technisch ist es ok. Aber ich würde keine OR-Mapped-Objekte bearbeiten. Sie können zu viele verschiedene Zustände haben, die Sie bei Operationen an ihnen sicherlich nicht berücksichtigen ...

OR-zugeordnete Objekte können vorübergehend sein (vorausgesetzt, JPA), dauerhaft getrennt, unverändert, unverändert, dauerhaft getrennt, dauerhaft verbunden geändert werden. Wenn Sie die Bean-Validierung für diese Objekte verwenden, können Sie nicht sehen, ob das Objekt überprüft wurde oder nicht. Immerhin können Sie mehr als 10 verschiedene Zustände identifizieren, die das OR-Mapped-Objekt haben kann. Behandeln alle Ihre Operationen diese Zustände richtig?

Dies ist sicherlich kein Problem, wenn Ihre Schnittstelle den Vertrag definiert und die Implementierung ihm folgt. Aber für OP-abgebildete Objekte habe ich begründete Zweifel, dass sie einen solchen Vertrag erfüllen können.