SOLIDOS vs métodos estáticos

11

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).

Wolfgang
fuente
2
Mientras no sea una variable estática, todo es genial. Los métodos estáticos son simples de probar y de rastrear
Codificador
3
¿Por qué no solo tener una clase de ProductsReviews? Entonces Producto y Revisión permanecen igual. O tal vez no lo entiendo.
ElGringoGrande
2
@Coder "¿Los métodos estáticos son fáciles de probar", realmente? No se pueden burlar, ver: googletesting.blogspot.com/2008/12/… para más detalles.
StuperUser
1
@StuperUser: No hay nada de lo que burlarse. Assert(5 = Math.Abs(-5));
Codificador
2
La prueba 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 en Abs()(o su código dependiente) y elimina los beneficios de diagnóstico de las pruebas unitarias.
StuperUser

Respuestas:

7

¿Qué es más importante: seguir los principios SÓLIDOS o tener una interfaz más simple?

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.

radarbob
fuente
1
+1 por notar que OCP es más un indicador de buena calidad, no una regla rápida para ningún código. Si encuentra que es difícil o imposible seguir adecuadamente el OCP, entonces esa es una señal de que su modelo necesita ser refactorizado para permitir una reutilización más flexible.
CodexArcanum
Elegí el ejemplo de Producto y Revisión para señalar que el Producto es una entidad central del proyecto, mientras que la Revisión es un mero complemento. Por lo tanto, el Producto ya existe, está terminado y la Revisión llega más tarde y debe presentarse sin abrir el código existente (incluido el Producto).
Wolfgang
10

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()es staticque no se puede probar CUT()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 en forProduct()(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.

StuperUser
fuente
1
Ese es un muy buen punto. En general, no para mí. ;-) No soy tan estricto acerca de escribir pruebas unitarias que cubren más de una clase. Todos van a la base de datos, eso está bien para mí. Por lo tanto, no me burlaré del objeto comercial o de su buscador.
Wolfgang
+1, ¡gracias por configurar la bandera roja para las pruebas! Estoy de acuerdo con @Wolfgang, por lo general no soy tan estricto, pero cuando necesito ese tipo de pruebas, realmente odio los métodos estáticos. Por lo general, si un método estático interactúa demasiado con sus parámetros o si interactúa con cualquier otro método estático, prefiero que sea un método de instancia.
Adriano Repetti
¿No depende esto completamente del idioma que se usa? El OP utilizó un ejemplo de Java, pero nunca mencionó un idioma en la pregunta, ni se especifica uno en las etiquetas.
Izkata
1
@StuperUser 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.
Izkata
1
@Izkata JS se escribe dinámicamente, por lo que no tiene 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.
StuperUser
4

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 - una TwitterReviewRetrievero un AmazonReviewRetrievero MultipleSourceReviewRetrievero 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 ProductWithReviewssi realmente quisieras ser pedante con tus principios SÓLIDOS, pero esto sería lo suficientemente bueno para mí).

Lunivore
fuente
Suena como el patrón DAO que es muy común en el software orientado a servicios / componentes. Me gusta esta idea porque expresa que recuperar objetos no es responsabilidad de estos objetos. Sin embargo, dado que prefiero una forma orientada a objetos sobre el servicio orientado.
Wolfgang
1
El uso de una interfaz como la IRetrieveReviewsdeja 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.
Lunivore
Sí, entonces sería una implementación del patrón de estrategia. Lo cual sería un argumento para una tercera clase. (A) y (B) no apoyarían esto. El buscador definitivamente usará un ORM, por lo que no hay razón para reemplazar el algoritmo. Lo siento si no estaba claro sobre esto en mi pregunta.
Wolfgang
3

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.

ElGringoGrande
fuente
No estoy de acuerdo en que tener el método de Revisión violaría SRP. En Producto, lo haría pero no en Revisión. Mi problema fue que es estático. Si moviera el método a alguna tercera clase, todavía sería estático y tendría el parámetro del producto.
Wolfgang
Entonces, ¿estás diciendo que la única responsabilidad de la clase Review, la única razón para que cambie, es si se necesita cambiar el método estático del producto? ¿No hay otra funcionalidad en la clase Review?
ElGringoGrande
Hay muchas cosas en revisión. Pero creo que un forProduct (Producto) le quedaría muy bien. Encontrar Revisiones depende mucho de los atributos y la estructura de la Revisión (¿qué identificadores únicos, qué alcance cambian los atributos?). Sin embargo, el Producto no sabe y no debe saber acerca de los Comentarios.
Wolfgang
3

No pondría la funcionalidad 'Obtener comentarios para el producto' ni en la Productclase ni en la Reviewclase ...

Tienes un lugar donde recuperar tus productos, ¿verdad? Algo con GetProductById(int productId)y tal vez GetProductsByCategory(int categoryId)y así sucesivamente.

Del mismo modo, debe tener un lugar para recuperar sus comentarios, con un GetReviewbyId(int reviewId)y tal vez un GetReviewsForProduct(int productId).

Cuando quiero cambiar la forma en que se recopilan las revisiones de un producto, tengo que cambiar la clase de Producto.

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.

Eric King
fuente
Así es como lo manejaría, y estoy confundido (y preocupado sobre si mis propias ideas son correctas) por la falta de esta respuesta hasta su publicación. Claramente, ni la clase que representa un informe ni la representación de un producto deben ser responsables de recuperar el otro. Algún tipo de servicios de suministro de datos debería estar manejando esto.
CodexArcanum
@CodexArcanum Bueno, no estás solo. :-)
Eric King
2

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.

Adriano Repetti
fuente
También estaba pensando en esta idea de la semántica de ambos extremos y cuál es "más fuerte" para que reclame el buscador. Es por eso que se me ocurrió (B). También ayudaría a crear una jerarquía de "abstracción" de las clases de negocios. El producto era un simple objeto básico donde Review es uno superior. Por lo tanto, Review puede hacer referencia al Producto pero no al revés. De esta forma, se pueden evitar ciclos en las referencias entre las clases de negocios, lo que sería otro problema resuelto en mi lista.
Wolfgang
Creo que para un pequeño conjunto de clases podría ser bueno incluso proporcionar AMBOS métodos (A) y (B). Demasiadas veces la IU no se conoce bien al momento de escribir esta lógica.
Adriano Repetti
1

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.

pfries
fuente
No estoy seguro de querer esto. De esta manera, tengo el método en ambos lugares, lo cual es bueno porque otros desarrolladores no necesitan mirar en ambas clases para ver dónde lo pongo. Por otro lado, tendría ambas desventajas del método estático y el cambio de la clase de producto "cerrado". No estoy seguro de que la delegación ayude al problema de la fricción. Si alguna vez hubo una razón para cambiar el buscador, seguramente dará como resultado el cambio de la firma y, por lo tanto, un cambio en el Producto nuevamente.
Wolfgang
La clave de OCP es que puede reemplazar implementaciones sin cambiar su código. En la práctica, esto está estrechamente relacionado con la sustitución de Liskov. Una implementación debe ser sustituible por otra. Es posible que tenga DatabaseReviews y SoapReviews como clases diferentes que implementan IGetReviews.getReviews y getReviewsForProducts. Luego, su sistema está abierto para la extensión (cambiando la forma en que se obtienen las revisiones) y cerrado para la modificación (las dependencias en IGetReviews no se rompen). Eso es lo que quiero decir con gestión de dependencia. En su caso, tendrá que modificar su código para cambiar su comportamiento.
pfries
¿No es ese el principio de inversión de dependencia (DIP) que estás describiendo?
Wolfgang
Son principios relacionados. Su punto de sustitución se logra mediante DIP.
pfries
1

Tengo una opinión ligeramente diferente sobre esto que la mayoría de las otras respuestas. Creo que Producty Reviewson 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:

interface IProductRepository
{
    void SaveNewProduct(IProduct product);
    IProduct GetProductById(ProductId productId);
    bool TryGetProductByName(string name, out IProduct product);
}

interface IProduct
{
    ProductId Id { get; }
    string Name { get; }
}

class ExistingProduct : IProduct
{
    public ProductId Id { get; private set; }
    public string Name { get; private set; }
}

Entonces su actual ProductRepositorydevolvería un ExistingProductpara el GetProductByProductIdmétodo, etc.

Ahora está siguiendo el principio de responsabilidad única (todo lo que hereda de lo que IProductse está aferrando al estado, y lo que sea que herede IProductRepositoryes 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. :)

Scott Whitlock
fuente
0

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

new ReviewService().GetReviews(productID);

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.

Tom Clarkson
fuente