(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 HolidayDiscount
y RentalDiscount
que representan descuentos por duración como "si dura al menos numberOfDays
, percent
se 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 HolidayOffer
tiene 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?
fuente
Respuestas:
Yo diría que su diseño está un poco equivocado.
Una posible solución sería crear una interfaz llamada
IDiscountCalculator
.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.
fuente
HolidayDiscount
eRentalDiscount
implementarIDiscountCalculator
, porque no son calculadoras de descuento. El cálculo se realiza en ese otro métodogetMaxApplicableLengthDiscount
.HolidayDiscount
yRentalDiscount
son descuentos. Un descuento no se calcula, es un tipo de descuento aplicable que se calcula, o simplemente se selecciona entre varios descuentos.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.
fuente