Aquí hay un problema con el que me encuentro con frecuencia: que haya un proyecto de tienda web que tenga una clase de Producto. Quiero agregar una función que permita a los usuarios publicar comentarios en un producto. Entonces tengo una clase Review que hace referencia a un producto. Ahora necesito un método que enumere todas las revisiones de un producto. Hay dos posibilidades:
(UNA)
public class Product {
...
public Collection<Review> getReviews() {...}
}
(SI)
public class Review {
...
static public Collection<Review> forProduct( Product product ) {...}
}
Al mirar el código, elegiría (A): no es estático y no necesita un parámetro. Sin embargo, siento que (A) viola el Principio de Responsabilidad Única (SRP) y el Principio Abierto-Cerrado (OCP) mientras que (B) no:
(SRP) Cuando quiero cambiar la forma en que se recopilan las revisiones de un producto, tengo que cambiar la clase de Producto. Pero solo debe haber una razón para cambiar la clase de Producto. Y ciertamente no son las reseñas. Si empaqueto todas las funciones que tienen algo que ver con los productos en el Producto, pronto saldrán ruidosamente.
(OCP) Tengo que cambiar la clase de Producto para ampliarla con esta función. Creo que esto viola la parte del principio "Cerrado por cambio". Antes de recibir la solicitud del cliente para implementar las revisiones, consideré el Producto como terminado y lo "cerré".
¿Qué es más importante: seguir los principios SÓLIDOS o tener una interfaz más simple?
¿O estoy haciendo algo mal aquí por completo?
Resultado
¡Guau, gracias por todas tus excelentes respuestas! Es difícil elegir uno como respuesta oficial.
Permítanme resumir los principales argumentos de las respuestas:
- pro (A): OCP no es una ley y la legibilidad del código también es importante.
- pro (A): la relación de la entidad debe ser navegable. Ambas clases pueden saber sobre esta relación.
- pro (A) + (B): haga ambas cosas y delegue en (A) a (B) para que sea menos probable que se vuelva a cambiar el Producto.
- pro (C): coloca los métodos del buscador en tercera clase (servicio) donde no es estático.
- contra (B): impide la burla en las pruebas.
Algunas cosas adicionales que mis universidades en el trabajo contribuyeron:
- pro (B): nuestro marco ORM puede generar automáticamente el código para (B).
- pro (A): por razones técnicas de nuestro marco ORM, será necesario cambiar la entidad "cerrada" en algunos casos, independientemente de dónde vaya el buscador. Por lo tanto, no siempre podré apegarme a SOLID.
- contra (C): mucho alboroto ;-)
Conclusión
Estoy usando ambos (A) + (B) con delegación para mi proyecto actual. Sin embargo, en un entorno orientado a servicios, iré con (C).
fuente
Assert(5 = Math.Abs(-5));
Abs()
no es el problema, probar algo que depende de él lo es. No tiene una costura para aislar el Código dependiente bajo prueba (CUT) para usar un simulacro. Esto significa que no puede probarlo como una unidad atómica y todas sus pruebas se convierten en pruebas de integración que prueban la lógica de la unidad. Una falla en una prueba podría estar en CUT o enAbs()
(o su código dependiente) y elimina los beneficios de diagnóstico de las pruebas unitarias.Respuestas:
Interfaz frente a SOLID
Estos no son mutuamente excluyentes. La interfaz debe expresar la naturaleza de su modelo de negocio idealmente en términos de modelo de negocio. Los principios SOLID es un Koan para maximizar la mantenibilidad del código orientado a objetos (y me refiero a "mantenibilidad" en el sentido más amplio). El primero admite el uso y la manipulación de su modelo de negocio y el segundo optimiza el mantenimiento del código.
Principio abierto / cerrado
"¡No toques eso!" Es una interpretación demasiado simplista. Y suponiendo que nos referimos a "la clase" es arbitrario, y no necesariamente correcto. Más bien, OCP significa que usted ha diseñado su código de tal manera que modificar su comportamiento no requiere (no debería) requerir que modifique directamente el código de trabajo existente. Además, no tocar el código en primer lugar es la forma ideal de preservar la integridad de las interfaces existentes; Este es un corolario significativo de OCP en mi opinión.
Finalmente veo OCP como un indicador de la calidad del diseño existente. Si me encuentro descifrando clases (o métodos) abiertos con demasiada frecuencia, y / o sin razones realmente sólidas (ja, ja) para hacerlo, entonces esto puede estar diciéndome que tengo un mal diseño (y / o no saber codificar OO).
Dale tu mejor oportunidad, tenemos un equipo de médicos esperando
Si su análisis de requisitos le indica que necesita expresar la relación de Revisión del producto desde ambas perspectivas, hágalo.
Por lo tanto, Wolfgang, puede tener una buena razón para modificar esas clases existentes. Dados los nuevos requisitos, si una Revisión es ahora una parte fundamental de un Producto, si cada extensión del Producto necesita una Revisión, si al hacerlo hace que el código del cliente sea expresivo de manera apropiada, integrelo en el Producto.
fuente
SÓLIDOS son pautas, por lo que influyen en la toma de decisiones en lugar de dictarlas.
Una cosa a tener en cuenta cuando se utilizan métodos estáticos es su impacto en la capacidad de prueba .
Las pruebas
forProduct(Product product)
no serán un problema.Probando algo que depende de ello será.
No tendrá una costura para aislar el Código dependiente bajo prueba (CUT) para usar un simulacro, ya que cuando la aplicación se está ejecutando, los métodos estáticos necesariamente existen.
Tengamos un método llamado
CUT()
que llamaforProduct()
Si
forProduct()
esstatic
que no se puede probarCUT()
como una unidad atómica y todas las pruebas se convierte en pruebas de integración que la lógica unidad de prueba.Una falla en una prueba de CUT podría ser causada por un problema en
CUT()
o enforProduct()
(o cualquiera de sus códigos dependientes) que elimine los beneficios de diagnóstico de las pruebas unitarias.Consulte esta excelente publicación de blog para obtener información más detallada: http://googletesting.blogspot.com/2008/12/static-methods-are-death-to-testability.html
Esto puede llevar a la frustración con el fracaso de las pruebas y el abandono de las buenas prácticas y los beneficios que las rodean.
fuente
If forProduct() is static you can't test CUT() as an atomic unit and all of your tests become integration tests that test unit logic.
: creo que Javascript y Python permiten que los métodos estáticos sean anulados / burlados. Sin embargo, no estoy 100% seguro.static
, lo simula con un cierre y el patrón singleton. Al leer en Python (especialmente stackoverflow.com/questions/893015/… ), debe heredar y ampliar. Anular no es burlarse; Parece que todavía no tienes una costura para probar el código como una unidad atómica.Si cree que el Producto es el lugar adecuado para encontrar las Revisiones del producto, siempre puede brindarle una clase de ayuda para que haga el trabajo por él. (Puede darse cuenta porque su empresa nunca hablará de una revisión, excepto en términos de un producto).
Por ejemplo, estaría tentado a inyectar algo que desempeñara el papel de un perro de revisión. Probablemente le daría la interfaz
IRetrieveReviews
. Puede poner esto en el constructor del producto (Inyección de dependencias). Si desea cambiar la forma de las revisiones se recuperan puede hacerlo fácilmente mediante la inyección de un colaborador diferente - unaTwitterReviewRetriever
o unAmazonReviewRetriever
oMultipleSourceReviewRetriever
o cualquier otra cosa que necesite.Ambos tienen ahora una responsabilidad única (ser el lugar de referencia para todo lo relacionado con el producto y recuperar revisiones, respectivamente), y en el futuro el comportamiento del producto con respecto a las revisiones puede modificarse sin cambiar realmente el producto (puede extenderlo como
ProductWithReviews
si realmente quisieras ser pedante con tus principios SÓLIDOS, pero esto sería lo suficientemente bueno para mí).fuente
IRetrieveReviews
deja de estar orientada al servicio: no determina qué recibe las revisiones, ni cómo ni cuándo. Tal vez es un servicio con muchos métodos para cosas como esta. Tal vez es una clase que hace eso. Tal vez es un repositorio, o hace una solicitud HTTP a un servidor. Usted no sabe No deberías saberlo. Ese es el punto.Tendría una clase de ProductsReview. Dices que Review es nuevo de todos modos. Eso no significa que pueda ser cualquier cosa. Todavía debe tener una sola razón para cambiar. Si cambia la forma en que obtiene las revisiones por cualquier razón, tendría que cambiar la clase Revisar.
Eso no es correcto
Está poniendo el método estático en la clase Review porque ... ¿por qué? ¿No es eso con lo que estás luchando? ¿No es ese el problema?
Entonces no lo hagas. Haga una clase cuya única responsabilidad sea obtener las revisiones de los productos. Luego puede subclasificarlo a ProductReviewsByStartRating lo que sea. O subclasifíquelo para obtener reseñas de una clase de productos.
fuente
No pondría la funcionalidad 'Obtener comentarios para el producto' ni en la
Product
clase ni en laReview
clase ...Tienes un lugar donde recuperar tus productos, ¿verdad? Algo con
GetProductById(int productId)
y tal vezGetProductsByCategory(int categoryId)
y así sucesivamente.Del mismo modo, debe tener un lugar para recuperar sus comentarios, con un
GetReviewbyId(int reviewId)
y tal vez unGetReviewsForProduct(int productId)
.Si separa su acceso a datos de sus clases de dominio, no necesitará alterar ninguna clase de dominio cuando cambie la forma en que se recopilan las revisiones.
fuente
Los patrones y principios son pautas, no reglas escritas en la piedra. En mi opinión, la pregunta no es si es mejor seguir los principios SÓLIDOS o mantener una interfaz más simple. Lo que debe preguntarse es qué es más legible y comprensible para la mayoría de las personas. A menudo, esto significa que debe estar lo más cerca posible del dominio.
En este caso, preferiría la solución (B) porque para mí el punto de partida es el Producto, no la Revisión, pero imagina que estás escribiendo un software para administrar las revisiones. En ese caso, el centro es la Revisión, por lo que la solución (A) puede ser preferible.
Cuando tengo muchos métodos como este ("conexiones" entre clases), los elimino todos y creo una (o más) nueva clase estática para organizarlos. Por lo general, puede verlos como consultas o como un repositorio.
fuente
Su producto puede simplemente delegar a su método de revisión estático, en cuyo caso está proporcionando una interfaz conveniente en una ubicación natural (Product.getReviews) pero los detalles de su implementación están en Review.getForProduct.
SÓLIDOS son pautas y deben dar como resultado una interfaz simple y sensata. Alternativamente, podría derivar SOLID de interfaces simples y sensibles. Se trata de la gestión de dependencias dentro del código. El objetivo es minimizar las dependencias que crean fricción y crean barreras al cambio inevitable.
fuente
Tengo una opinión ligeramente diferente sobre esto que la mayoría de las otras respuestas. Creo que
Product
yReview
son básicamente de transferencia de datos de objetos (DTO). En mi código trato de hacer que mis DTO / Entidades eviten tener comportamientos. Son solo una buena API para almacenar el estado actual de mi modelo.Cuando habla de OO y SOLID, generalmente está hablando de un "objeto" que no representa el estado (necesariamente), sino que representa un tipo de servicio que responde preguntas por usted o al que puede delegar parte de su trabajo . Por ejemplo:
Entonces su actual
ProductRepository
devolvería unExistingProduct
para elGetProductByProductId
método, etc.Ahora está siguiendo el principio de responsabilidad única (todo lo que hereda de lo que
IProduct
se está aferrando al estado, y lo que sea que heredeIProductRepository
es responsable de saber cómo persistir y rehidratar su modelo de datos).Si cambia el esquema de su base de datos, puede cambiar la implementación de su repositorio sin cambiar sus DTO, etc.
En resumen, supongo que no elegiría ninguna de sus opciones. :)
fuente
Los métodos estáticos pueden ser más difíciles de probar, pero eso no significa que no pueda usarlos, solo necesita configurarlos para que no necesite probarlos.
Haga ambos productos, obtenga revisiones y revise los métodos de una línea que son algo así como
ReviewService contiene todo el código más complejo y tiene una interfaz diseñada para la capacidad de prueba, pero que no está expuesta directamente al usuario.
Si debe tener una cobertura del 100%, haga que sus pruebas de integración llamen a los métodos de clase de producto / revisión.
Podría ser útil si piensa en el diseño público de API en lugar del diseño de clase; en ese contexto, una interfaz simple con agrupación lógica es lo único que importa; la estructura del código real solo es importante para los desarrolladores.
fuente