Cómo argumentar en contra de esta mentalidad "completamente pública" del diseño de clase de objeto comercial

9

Estamos haciendo muchas pruebas unitarias y refactorizando nuestros objetos comerciales, y parece que tengo opiniones muy diferentes sobre el diseño de la clase que otros pares.

Una clase de ejemplo de la que no soy fanático:

public class Foo
{
    private string field1;
    private string field2;
    private string field3;
    private string field4;
    private string field5;

    public Foo() { }

    public Foo(string in1, string in2)
    {
        field1 = in1;
        field2 = in2;
    }

    public Foo(string in1, string in2, string in3, string in4)
    {
        field1 = in1;
        field2 = in2;
        field3 = in3;
    }

    public Prop1
    { get { return field1; } }
    { set { field1 = value; } }

    public Prop2
    { get { return field2; } }
    { set { field2 = value; } }

    public Prop3
    { get { return field3; } }
    { set { field3 = value; } }

    public Prop4
    { get { return field4; } }
    { set { field4 = value; } }

    public Prop5
    { get { return field5; } }
    { set { field5 = value; } }

}

En la clase "real", no todas son cadenas, pero en algunos casos tenemos 30 campos de respaldo para propiedades completamente públicas.

Me gusta esta clase, y no sé si estoy siendo quisquilloso. Algunas cosas notables:

  • Los campos de respaldo privados sin lógica en las propiedades, parecen innecesarios e hinchan la clase
  • Múltiples constructores (algo bien) pero junto con
  • Todas las propiedades tienen un setter público, no soy un fanático.
  • Potencialmente, a las propiedades no se les asignaría un valor debido al constructor vacío, si una persona que llama no lo sabe, podría obtener algunos comportamientos no deseados y difíciles de probar.
  • ¡Son demasiadas propiedades! (en el caso 30)

Me resulta mucho más difícil saber realmente en qué estado se encuentra el objeto Fooen un momento dado, como implementador. El argumento se hizo "podría no tener la información necesaria para establecer Prop5en el momento de la construcción del objeto. Ok, supongo que puedo entender eso, pero si ese es el caso, haga Prop5público solo el setter, no siempre hasta 30 propiedades en una clase.

¿Estoy siendo quisquilloso y / o loco por querer una clase que sea "fácil de usar" en lugar de ser "fácil de escribir (todo público)"? Clases como las anteriores me gritan, no sé cómo se va a usar esto, así que voy a hacer todo público por si acaso .

Si no estoy siendo muy exigente, ¿cuáles son buenos argumentos para combatir este tipo de pensamiento? No soy muy bueno para articular argumentos, ya que me siento muy frustrado al tratar de expresar mi punto de vista (no intencionalmente, por supuesto).

Kritner
fuente
3
Un objeto siempre debe estar en un estado válido; es decir, no debería necesitar tener objetos parcialmente instanciados . Parte de la información no es parte del estado central de un objeto, mientras que otra información sí lo es (por ejemplo, una mesa debe tener patas pero la tela es opcional); Se puede proporcionar información opcional después de la creación de instancias, la información central se debe proporcionar en la creación de instancias. Si alguna información es esencial pero no se conoce en la instanciación, entonces diría que las clases necesitan refactorización. Es difícil decir más sin conocer el contexto de la clase.
Marvin
1
Al decir "es posible que no tengamos la información necesaria para establecer la Prop5 en el momento de la construcción del objeto", me hace preguntar "¿Está diciendo que habrá algunas veces que tendrá esa información en el momento de la construcción y otras cuando no lo hará? tiene esa información? Me hace preguntarme si en realidad hay dos cosas diferentes que esta clase está tratando de representar, o tal vez si esta clase debería dividirse en clases más pequeñas. Pero si se hace "por si acaso", eso también es malo, en mi opinión; eso me dice que no existe un diseño claro sobre cómo se crean / completan los objetos.
Aprendiz del Dr. Wily
3
Para los campos de respaldo privados sin lógica en las propiedades, ¿hay alguna razón por la que no esté utilizando las propiedades automáticas ?
Carson63000
2
@Kritner, el punto principal de las propiedades automáticas es que si necesita una lógica real en el futuro, puede agregarla sin problemas en lugar de la automática geto set:-)
Carson63000

Respuestas:

10

Las clases completamente públicas tienen una justificación para ciertas situaciones, así como el otro extremo, clases con un solo método público (y probablemente muchos métodos privados). Y clases con algunos métodos públicos, algunos privados también.

Todo depende del tipo de abstracción que vaya a modelar con ellos, qué capas tiene en su sistema, el grado de encapsulación que necesita en las diferentes capas y (por supuesto) a qué escuela de pensamiento llega el autor de la clase desde. Puede encontrar todos estos tipos en código SOLIDO.

Hay libros completos escritos sobre cuándo preferir qué tipo de diseño, por lo que no voy a enumerar ninguna regla aquí al respecto, el espacio en esta sección no sería suficiente. Sin embargo, si tiene un ejemplo del mundo real para una abstracción que le gusta modelar con una clase, estoy seguro de que la comunidad aquí lo ayudará a mejorar el diseño.

Para abordar sus otros puntos:

  • "campos de respaldo privados sin lógica en las propiedades": Sí, tiene razón, para captadores y establecedores triviales esto es simplemente "ruido" innecesario. Para evitar este tipo de "hinchazón", C # tiene una sintaxis abreviada para los métodos get / set de propiedad:

Entonces en lugar de

   private string field1;
   public string Prop1
   { get { return field1; } }
   { set { field1 = value; } }

escribir

   public string Prop1 { get;set;}

o

   public string Prop1 { get;private set;}
  • "Constructores múltiples": eso no es un problema en sí mismo. Se produce un problema cuando hay una duplicación de código innecesaria allí, como se muestra en su ejemplo, o la jerarquía de llamadas está enrevesada. Esto se puede resolver fácilmente refactorizando partes comunes en una función separada y organizando la cadena de constructor de manera unidireccional

  • "Potencialmente no se asignaría un valor a las propiedades debido al constructor vacío": en C #, cada tipo de datos tiene un valor predeterminado claramente definido. Si las propiedades no se inicializan explícitamente en un constructor, se les asigna este valor predeterminado. Si esto se usa intencionalmente , está perfectamente bien, por lo que un constructor vacío podría estar bien si el autor sabe lo que está haciendo.

  • "¡Son demasiadas propiedades! (En el caso de 30)": sí, si usted es libre de diseñar una clase de este tipo de manera ecológica, 30 son demasiadas, estoy de acuerdo. Sin embargo, no todos tenemos este lujo (¿no escribiste en el comentario a continuación es un sistema heredado?). A veces tiene que asignar registros de una base de datos, archivo o datos existentes de una API de terceros a su sistema. Entonces, para estos casos, 30 atributos pueden ser algo con lo que uno tiene que vivir.

Doc Brown
fuente
Gracias, sí, sé que los POCO / POJO tienen su lugar, y mi ejemplo es bastante abstracto. Sin embargo, no creo que pueda ser más específico sin entrar en una novela.
Kritner
@Kritner: mira mi edición
Doc Brown
Sí, por lo general, cuando estoy creando una nueva clase, voy por la ruta de la propiedad automática. Tener los campos de respaldo privados "solo porque" es (en mi opinión) innecesario e incluso causa problemas. Cuando tiene ~ 30 propiedades para una clase y más de 100 clases, no es descabellado decir que habrá alguna configuración de Propiedades u obtenida del campo de respaldo incorrecto ... De hecho, ya me he encontrado con esto varias veces en nuestra refactorización. :)
Kritner
gracias, el valor predeterminado para la cadena es null¿no? así que si una de las propiedades que realmente se usa en a .ToUpper(), pero nunca se establece un valor para ella, generaría una excepción de tiempo de ejecución. Este es otro ejemplo bien por qué los necesarios datos para la clase deben tener que ser establecido durante la construcción de objetos. No solo se lo dejo al usuario. Gracias
Kritner
Además de demasiadas propiedades sin establecer, el principal problema con esta clase es que tiene lógica cero (ZRP) y es el arquetipo de un modelo anémico. Sin una necesidad de esto que pueda expresarse en palabras, como un DTO, es un diseño pobre.
user949300
2
  1. Al decir que "los campos de respaldo privados sin lógica en las propiedades, parecen innecesarios e hinchan la clase" ya tienes un argumento decente.

  2. No parece que varios constructores sean el problema aquí.

  3. En cuanto a tener todas las propiedades como públicas ... Tal vez piense de esta manera, si alguna vez quisiera sincronizar el acceso a todas estas propiedades entre subprocesos, lo pasaría mal porque podría tener que escribir código de sincronización en todas partes usado. Sin embargo, si todas las propiedades estaban encerradas por getters / setters, entonces podría construir fácilmente la sincronización en la clase.

  4. Creo que cuando dices "comportamiento difícil de probar" el argumento habla por sí mismo. Es posible que su prueba tenga que verificar si no es nula o algo así (en cada lugar donde se usa la propiedad). Realmente no sé porque no sé cómo se ve su prueba / aplicación.

  5. Tienes demasiadas propiedades, ¿podrías intentar usar la herencia (si las propiedades son lo suficientemente similares) y luego tener una lista / matriz usando genéricos? Luego, podría escribir getters / setters para acceder a la información y simplificar la forma en que interactuará con todas las propiedades.

Fisgonear
fuente
1
Gracias - # 1 y # 4 Definitivamente me siento más cómodo con mi argumento. No estoy muy seguro de lo que quieres decir en el n. ° 3. # 5 la mayoría de las propiedades en las clases constituyen la clave primaria en el db. Usamos claves competitivas, a veces hasta 8 columnas, pero ese es otro problema. Estaba pensando en tratar de poner las partes que componen la clave en su propia clase / estructura, lo que eliminaría muchas propiedades (al menos en esta clase), pero esta es una aplicación heredada con miles de líneas de código con múltiples personas que llaman Así que supongo que tengo mucho en qué pensar :)
Kritner
Eh Si la clase fuera inmutable, no habría razón para escribir ningún código de sincronización dentro de la clase.
RubberDuck
@RubberDuck ¿Es esta clase inmutable? ¿Qué quieres decir?
Snoop
3
Definitivamente no es inmutable @StevieV. Cualquier clase con establecedores de propiedades es mutable.
RubberDuck
1
@Kritner ¿Cómo se utilizan las propiedades como claves principales? Eso presumiblemente requiere cierta lógica (verificaciones nulas) o reglas (por ejemplo, NAME tiene prioridad sobre la EDAD). Según OOP, este código pertenece a esta clase. ¿Podrías usar eso como argumento?
user949300
2

Como me dijiste, Fooes un objeto comercial. Debe encapsular algún comportamiento en los métodos de acuerdo con su dominio y sus datos deben permanecer tan encapsulados como sea posible.

En el ejemplo que muestra, se Fooparece más a un DTO y va en contra de todos los principios de OOP (ver Modelo de dominio anémico ).

Como mejoras sugeriría:

  • Haga que esta clase sea lo más inmutable posible. Como dijiste, quieres hacer algunas pruebas unitarias. Una capacidad obligatoria para las pruebas unitarias es el determinismo , y el determinismo se puede lograr mediante la inmutabilidad porque resuelve los problemas de efectos secundarios .
  • Divide esta clase de dios en varias clases que solo hacen 1 cosa cada una (ver SRP ). Prueba de unidad de una clase de 30 atributos en realmente un PITA .
  • Elimine la redundancia del constructor teniendo solo 1 constructor principal y los demás llamando al constructor principal.
  • Elimina getters innecesarios, dudo seriamente que todos los getters sean realmente útiles.
  • Recupera la lógica de negocios en las clases de negocios, ¡aquí es donde pertenece!

Esta podría ser una posible clase refactorizada con estos comentarios:

public sealed class Foo
{
    private readonly string field1;
    private readonly string field2;
    private readonly string field3;
    private readonly string field4;

    public Foo() : this("default1", "default2")
    { }

    public Foo(string in1, string in2) : this(in1, in2, "default3", "default4")
    { }

    public Foo(string in1, string in2, string in3, string in4)
    {
        field1 = in1;
        field2 = in2;
        field3 = in3;
        field4 = in4;
    }

    //Methods with business logic

    //Really needed ?
    public Prop1
    { get; }

    //Really needed ?
    public Prop2
    { get; }

    //Really needed ?
    public Prop3
    { get; }

    //Really needed ?
    public Prop4
    { get; }

    //Really needed ?
    public Prop5
    { get; }
}
Manchado
fuente
2

Cómo argumentar en contra de esta mentalidad "completamente pública" del diseño de clase de objeto comercial

  • "Hay varios métodos dispersos en la (s) clase (s) del cliente que hacen más que 'simple deber de DTO'. Van juntos".
  • "Puede ser un 'DTO' pero tiene una identidad comercial: necesitamos anular Equals".
  • "Necesitamos clasificar esto, necesitamos implementar IComparable"
  • "Estamos encadenando varias de estas propiedades. Anulemos ToString para que cada cliente no tenga que escribir este código".
  • "Tenemos varios clientes que hacen lo mismo con esta clase. Necesitamos SECAR el código".
  • "El cliente está manipulando una colección de estas cosas. Es obvio que debería ser una clase de colección personalizada; y muchos de los puntos anteriores harán que esa colección personalizada sea más funcional que la actual".
  • "¡Mira toda la tediosa, expuesta, manipulación de cadenas! Encapsular eso en métodos relevantes para los negocios aumentará nuestra productividad de codificación".
  • "Reunir estos métodos en la clase los hará verificables porque no tenemos que lidiar con la clase de cliente más compleja".
  • "Ahora podemos probar esos métodos refactorizados. Tal como está, por razones x, y, z el cliente no es verificable".

  • En la medida en que se pueda hacer cualquiera de los argumentos anteriores, en conjunto:

    • La funcionalidad se vuelve reutilizable.
    • Esta seco
    • Está desacoplado de los clientes.
    • la codificación contra la clase es más rápida y menos propensa a errores.
  • "Una clase bien hecha oculta el estado y expone la funcionalidad. Esta es la propuesta inicial para diseñar clases OO".
  • "El resultado final hace un trabajo mucho mejor al expresar el dominio comercial; las distintas entidades y su interacción explícita".
  • "Esta funcionalidad de 'gastos generales' (es decir, no requisitos funcionales explícitos, pero debe hacerse) se destaca claramente y se adhiere al Principio de Responsabilidad Única.
radarbob
fuente