Código limpio: consecuencias de métodos cortos con pocos parámetros.

15

Recientemente, durante una revisión de código, me encontré con un código, escrito por un nuevo colega, que contiene un patrón con olor. Sospecho que las decisiones de mi colega se basan en reglas propuestas por el famoso libro Clean Code (y quizás también por otros libros similares).

Entiendo que el constructor de la clase es completamente responsable de la creación de un objeto válido y que su tarea principal es la asignación de las propiedades (privadas) de un objeto. Por supuesto, podría ocurrir que los valores de propiedad opcionales se puedan establecer por métodos distintos al constructor de la clase, pero tales situaciones son bastante raras (aunque no necesariamente incorrectas, siempre que el resto de la clase tenga en cuenta la opcionalidad de dicha propiedad). Esto es importante porque permite garantizar que el objeto esté siempre en un estado válido.

Sin embargo, en el código que encontré, la mayoría de los valores de propiedad se establecen en realidad por otros métodos distintos al constructor. Los valores que resultan de los cálculos se asignan a las propiedades que se utilizarán dentro de varios métodos privados en toda la clase. Aparentemente, el autor usa las propiedades de la clase como si fueran variables globales que deberían ser accesibles en toda la clase, en lugar de parametrizar estos valores para las funciones que los necesitan. Además, los métodos de la clase deben llamarse en un orden específico, porque de lo contrario la clase no hará mucho.

Sospecho que este código se inspiró en los consejos para mantener los métodos cortos (<= 5 líneas de código), para evitar listas de parámetros grandes (<3 parámetros) y que los constructores no deben trabajar (como realizar algún tipo de cálculo) eso es esencial para la validez del objeto).

Ahora, por supuesto, podría presentar un caso en contra de este patrón si puedo demostrar que todo tipo de errores indefinidos potencialmente surgen cuando los métodos no se invocan en un orden específico. Sin embargo, predigo que la respuesta a esto será agregar validaciones que verifiquen que las propiedades deben establecerse una vez que se invocan los métodos que necesitan que se establezcan esas propiedades.

Sin embargo, preferiría proponer cambiar completamente el código, de modo que la clase se convierta en una copia original de un objeto real, en lugar de una serie de métodos que deberían llamarse (de manera procesal) en un orden específico.

Siento que el código que encontré huele. De hecho, creo que existe una distinción bastante clara en cuanto a cuándo guardar un valor en una propiedad de clase y cuándo ponerlo en un parámetro para que lo use un método diferente; realmente no creo que puedan ser alternativas entre sí . Estoy buscando las palabras para esta distinción.

usuario2180613
fuente
66
1. Jugar al abogado del diablo por un momento ... ¿Funciona realmente el código? Porque los objetos de transferencia de datos son una técnica perfectamente válida, y si eso es todo esto ...
Robert Harvey
77
2. Si le faltan las palabras para describir el problema, entonces no tiene suficiente experiencia para refutar la posición de su colega.
Robert Harvey
44
3. Si tiene algún código de trabajo que puede publicar, publíquelo en Revisión de código y déjelos echarle un vistazo. De lo contrario, esto es solo una generalidad errante.
Robert Harvey
55
@RobertHarvey "Los valores que resultan de los cálculos se asignan a las propiedades que se utilizarán dentro de varios métodos privados en toda la clase" no me parece un DTO que se respete a sí mismo. Estoy de acuerdo en que un poco más de especificidad sería útil.
topo Restablecer Monica
44
Aparte: Parece que alguien no ha leído Clean Code antes de criticarlo. Simplemente lo escaneé de nuevo, y no pude encontrar ningún lugar donde sugiera que "los constructores no deberían funcionar" (algunos ejemplos de hecho realizan trabajo), y la solución sugerida para evitar demasiados parámetros es crear un objeto de parámetro relacionado con la consolidación grupos de parámetros, no bastardear sus funciones. Y el libro no sugiere refactorización código para evitar dependencias temporales entre los métodos. Creo que su sesgo en contra de algunos de sus estilos de código preferidos han influido en su percepción del libro.
Eric King

Respuestas:

13

Como alguien que ha leído Clean Code y ha visto la serie Clean Coders, varias veces, y a menudo enseña y entrena a otras personas a escribir código más limpio, puedo garantizar que sus observaciones son correctas: las métricas que señala se mencionan en el libro .

Sin embargo, el libro continúa para hacer otros puntos que también deberían aplicarse junto con las pautas que usted señaló. Estos fueron aparentemente ignorados en el código con el que está tratando. Esto puede haber sucedido porque su colega todavía está en la fase de aprendizaje, en cuyo caso, por mucho que sea necesario señalar los olores de su código, es bueno recordar que lo están haciendo de buena voluntad, aprendiendo y tratando para escribir mejor código.

Clean Code propone que los métodos deben ser cortos, con la menor cantidad de argumentos posibles. Pero siguiendo esas pautas, propone que debemos seguir los principios S OLID, aumentar la cohesión y reducir el acoplamiento .

La S en SOLID significa Principio de responsabilidad única, que establece que un objeto debe ser responsable de una sola cosa. "Cosa" no es un término muy preciso, por lo que las descripciones de este principio varían enormemente. Sin embargo, el tío Bob, el autor de Clean Code, es también la persona que acuñó este principio, describiéndolo como: "Reúna las cosas que cambian por las mismas razones. Separe las cosas que cambian por diferentes razones". Continúa diciendo lo que quiere decir con razones para cambiar aquí y aquí(Una explicación más larga aquí sería demasiado). Si este principio se aplicó a la clase con la que está tratando, es muy probable que las piezas que se ocupan de los cálculos se separen de las que se ocupan del estado de espera, dividiendo la clase en dos o más, dependiendo de cuántas razones para cambiar esos cálculos tienen.

Además, las clases Clean deben ser coherentes , lo que significa que la mayoría de sus métodos utilizan la mayoría de sus atributos. Como tal, una clase de máxima cohesión es aquella en la que todos los métodos usan todos sus atributos; Como ejemplo, en una aplicación gráfica puede tener una Vectorclase con atributos Point ay Point b, donde están los únicos métodos scaleBy(double factor)y printTo(Canvas canvas), ambos operando en ambos atributos. En contraste, una clase mínimamente cohesiva es aquella en la que cada atributo se usa en un solo método, y nunca se usa más de un atributo por cada método. En promedio, una clase presentes no cohesivos "grupos" de partes cohesivos - es decir, algunos métodos utilizan atributos a, by c, mientras que el uso resto cyd - lo que significa que si dividimos la clase en dos, terminamos con dos objetos cohesivos.

Finalmente, las clases Clean deberían reducir el acoplamiento tanto como sea posible. Si bien hay muchos tipos de acoplamiento que vale la pena discutir aquí, parece que el código en cuestión sufre principalmente de acoplamiento temporal , donde, como señaló, los métodos del objeto solo funcionarán como se espera cuando se invocan en el orden correcto. Y al igual que las dos pautas mencionadas anteriormente, las soluciones a esto generalmente implican dividir la clase en dos o más objetos cohesivos . La estrategia de división en este caso generalmente involucra patrones como Builder o Factory, y en casos muy complejos, State-Machines.

El TL; DR: Las pautas del Código Limpio que siguió su colega son buenas, pero solo cuando también siguen los principios, prácticas y patrones restantes mencionados en el libro. La versión limpia de la "clase" que está viendo se dividiría en varias clases, cada una con una sola responsabilidad, métodos cohesivos y sin acoplamiento temporal. Este es el contexto donde los pequeños métodos y los argumentos de poco a nada tienen sentido.

MichelHenrich
fuente
1
Tanto usted como topo morto han escrito una buena respuesta, pero solo puedo aceptar una. Me gusta que haya abordado SRP, cohesión y acoplamiento. Estos son términos útiles que puedo usar en la revisión del código. Dividir el objeto en objetos más pequeños con sus propias responsabilidades es obviamente el camino a seguir. Un método (no constructor) que inicializa los valores en un conjunto de propiedades de clase es un obvio que un nuevo objeto debería ser devuelto. Debería haberlo visto.
user2180613
1
SRP es LA directriz más importante; un anillo para gobernarlos a todos y todo eso. Bien hecho SRP naturalmente da como resultado métodos más cortos. Ejemplo: Tengo una clase frontal con solo 2 métodos públicos y unos 8 métodos no públicos. Ninguno es más de ~ 3 líneas; toda la clase es de unos 35 LOC. ¡Pero escribí esta clase al final! Para cuando se escribió todo el código subyacente, esta clase se escribió esencialmente y no tuve que, de hecho, no pude, hacer que los métodos fueran más grandes. En ningún momento dije "voy a escribir estos métodos en 5 líneas si me mata". Cada vez que aplica SRP, simplemente sucede.
radarbob
11

Entiendo que el constructor de la clase es completamente responsable de la creación de un objeto válido y que su tarea principal es la asignación de las propiedades (privadas) de un objeto.

Generalmente es responsable de poner el objeto en un estado válido inicial, sí; otras propiedades o métodos pueden cambiar el estado a otro estado válido.

Sin embargo, en el código que encontré, la mayoría de los valores de propiedad se establecen en realidad por otros métodos distintos al constructor. Los valores que resultan de los cálculos se asignan a las propiedades que se utilizarán dentro de varios métodos privados en toda la clase. Aparentemente, el autor usa las propiedades de la clase como si fueran variables globales que deberían ser accesibles en toda la clase, en lugar de parametrizar estos valores para las funciones que los necesitan. Además, los métodos de la clase deben llamarse en un orden específico, porque de lo contrario la clase no hará mucho.

Además de los problemas de legibilidad y mantenibilidad a los que alude, parece que hay varias etapas de flujo / transformación de datos en curso dentro de la propia clase, lo que puede indicar que la clase está incumpliendo el principio de responsabilidad única.

sospecha que este código se ha inspirado en el consejo de mantener métodos cortos (<= 5 líneas de código), para evitar listas de parámetros grandes (<3 parámetros) y que los constructores no deben trabajar (como realizar un cálculo de algún tipo que es esencial para la validez del objeto).

Seguir algunas pautas de codificación e ignorar otras a menudo conduce a un código tonto. Si queremos evitar que un constructor trabaje, por ejemplo, la forma más sensata sería hacer el trabajo antes de la construcción y pasar el resultado de ese trabajo al constructor. (Un argumento para ese enfoque podría ser que está evitando darle a su clase dos responsabilidades: el trabajo de su inicialización y su 'trabajo principal', sea lo que sea).

En mi experiencia, hacer que las clases y los métodos sean pequeños rara vez es algo que tengo que tener en cuenta como una consideración separada; más bien, se deduce de manera algo natural de la responsabilidad individual.

Sin embargo, preferiría proponer cambiar completamente el código, de modo que la clase se convierta en una copia original de un objeto real, en lugar de una serie de métodos que deberían llamarse (de manera procesal) en un orden específico.

Probablemente sea correcto hacerlo. No hay nada de malo en escribir código de procedimiento directo; Hay un problema al abusar del paradigma OO para escribir código de procedimiento ofuscado.

Creo que existe una distinción bastante clara sobre cuándo guardar un valor en una propiedad de clase y cuándo ponerlo en un parámetro para que lo use un método diferente; realmente no creo que puedan ser alternativas entre sí. Estoy buscando las palabras para esta distinción.

Por lo general , no debería poner un valor en un campo como una forma de pasarlo de un método a otro; Un valor en un campo debe ser una parte significativa del estado de un objeto en un momento dado. (Puedo pensar en algunas excepciones válidas, pero no en aquellas en las que dichos métodos son públicos o donde hay una dependencia del orden que un usuario de la clase debería tener en cuenta)

topo Restablecer Monica
fuente
2
voto a favor porque: 1. Enfatizar SRP. "... pequeños métodos ... siguen naturalmente" 2. Propósito del constructor - estado válido. 3. "Seguir algunas pautas de codificación e ignorar otras". Este es el Walking Dead de la codificación.
radarbob
6

Lo más probable es que estés criticando el patrón incorrecto aquí. Las funciones pequeñas y la baja aridad rara vez son problemáticas por sí mismas. El verdadero problema aquí es el acoplamiento que causa la dependencia del orden entre las funciones, así que busque formas de abordar eso sin desechar los beneficios de las funciones pequeñas.

El código habla más que las palabras. Las personas reciben este tipo de correcciones mucho mejor si realmente puede hacer parte de la refactorización y mostrar la mejora, tal vez como un ejercicio de programación en pareja. Cuando hago esto, a menudo encuentro que es más difícil de lo que pensaba hacer bien el diseño, equilibrando todos los criterios.

Karl Bielefeldt
fuente