¿Es una interfaz con solo getters un olor a código?

10

(He visto esta pregunta , pero la primera respuesta trata sobre las propiedades automáticas más que sobre el diseño, y la segunda dice que esconda el código de almacenamiento de datos del consumidor , lo que no estoy seguro es lo que quiero / mi código hace, así que me gustaría escuchar alguna otra opinión)

Tengo dos entidades muy similares HolidayDiscounty RentalDiscountque representan descuentos por duración como "si dura al menos numberOfDays, percentse aplica un descuento". Las tablas tienen fks para diferentes entidades principales y se usan en diferentes lugares, pero donde se usan, existe una lógica común para obtener el descuento máximo aplicable. Por ejemplo, a HolidayOffertiene una cantidad de HolidayDiscounts, y al calcular su costo, necesitamos calcular el descuento aplicable. Lo mismo para alquileres y RentalDiscounts.

Como la lógica es la misma, quiero mantenerla en un solo lugar. Eso es lo que hacen el siguiente método, predicado y comparador:

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

Como la única información necesaria es la cantidad de días, termino con una interfaz como

public interface LengthDiscount {

    Integer getNumberOfDays();
}

Y las dos entidades

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

La interfaz tiene un único método getter que implementan las dos entidades, que por supuesto funciona, pero dudo que sea un buen diseño. No representa ningún comportamiento, dado que mantener un valor no es una propiedad. Este es un caso bastante simple, tengo un par más de casos similares y más complejos (con 3-4 captadores).

Mi pregunta es, ¿es este un mal diseño? ¿Cuál es un mejor enfoque?

usuario3748908
fuente
44
El propósito de una interfaz es establecer un patrón de conexión, no representar el comportamiento. Ese deber recae en los métodos de implementación.
Robert Harvey
En cuanto a su implementación, hay una gran cantidad de repeticiones (código que en realidad no hace nada, aparte de proporcionar estructura). ¿Estás seguro de que lo necesitas todo?
Robert Harvey
Métodos de interfaz incluso son sólo 'captadores' no significa que no puede implementos 'set' en la aplicación (si todos tienen colocador, s todavía se puede utilizar un resumen)
рüффп

Respuestas:

5

Yo diría que su diseño está un poco equivocado.

Una posible solución sería crear una interfaz llamada IDiscountCalculator.

decimal CalculateDiscount();

Ahora cualquier clase que necesite proporcionar un descuento implementaría esta interfaz. Si el descuento usa varios días, que así sea, a la interfaz realmente no le importa, ya que son los detalles de implementación.

El número de días probablemente pertenece a algún tipo de clase base abstracta si todas las clases de descuento necesitan este miembro de datos. Si es específico de la clase, simplemente declare una propiedad a la que se pueda acceder de forma pública o privada según la necesidad.

Jon Raynor
fuente
1
No estoy seguro de estar completamente de acuerdo con tener HolidayDiscounte RentalDiscountimplementar IDiscountCalculator, porque no son calculadoras de descuento. El cálculo se realiza en ese otro método getMaxApplicableLengthDiscount. HolidayDiscounty RentalDiscountson descuentos. Un descuento no se calcula, es un tipo de descuento aplicable que se calcula, o simplemente se selecciona entre varios descuentos.
user3748908
1
Tal vez la interfaz debería llamarse IDiscountable. Cualquier clase que necesiten implementar puede. Si las clases de descuento de vacaciones / alquiler son solo DTO / POCO y almacenan el resultado en lugar de calcularlo, está bien.
Jon Raynor
3

Para responder a su pregunta: no puede concluir un olor a código si una interfaz solo tiene captadores.

Un ejemplo de una métrica difusa para olores de código son las interfaces hinchadas con muchos métodos (no lo hace si los captadores o no). Tienden a violar el principio de segmentación de la interfaz.

En el nivel semántico, el principio de responsabilidad única no debe ser violado también. Tiendo a ser violado si ves varios problemas diferentes manejados en el contrato de interfaz que no son un tema. Esto a veces es difícil de identificar y necesitas algo de experiencia para identificarlo.

Por otro lado, debe saber si su interfaz define configuradores. Eso es porque es probable que los setters cambien de estado, ese es su trabajo. La pregunta que debe hacerse aquí: ¿el objeto detrás de la interfaz hace una transición de un estado válido a otro estado válido? Pero eso no es principalmente un problema de la interfaz, sino la implementación del contrato de interfaz del objeto de implementación.

La única preocupación que tengo con su enfoque es que opere en asignaciones OR. En este pequeño caso, esto no es un problema. Técnicamente está bien. Pero no operaría en objetos con mapas OR. Pueden tener demasiados estados diferentes que seguramente no considerará en las operaciones realizadas en ellos ...

Los objetos O-Mapped pueden ser (presuponiendo JPA) transitorios, persistentes desconectados sin cambios, persistentes adjuntos sin cambios, persistentes desconectados cambiados, persistentes adjuntos cambiados ... si usa la validación de bean en esos objetos, no podrá ver si el objeto fue verificado o no. Después de todo, puede identificar más de 10 estados diferentes que puede tener el objeto O-Mapped. ¿Todas sus operaciones manejan estos estados correctamente?

Esto ciertamente no es un problema si su interfaz define el contrato y la implementación lo sigue. Pero para los objetos mapeados OR tengo dudas razonables de que pueda cumplir dicho contrato.

oopexpert
fuente