Línea adicional en bloque versus parámetro adicional en Código limpio

33

Contexto

En Clean Code , página 35, dice

Esto implica que los bloques dentro de las declaraciones if, las declaraciones else, las declaraciones while, etc. deben tener una línea de longitud. Probablemente esa línea debería ser una llamada de función. Esto no solo mantiene la función de cierre pequeña, sino que también agrega valor documental porque la función llamada dentro del bloque puede tener un nombre muy descriptivo.

Estoy completamente de acuerdo, eso tiene mucho sentido.

Más adelante, en la página 40, dice sobre argumentos de función

El número ideal de argumentos para una función es cero (niládico). Luego viene uno (monádico), seguido de cerca por dos (diádico). Se deben evitar tres argumentos (triádicos) siempre que sea posible. Más de tres (poliádico) requieren una justificación muy especial, y de todos modos no deberían usarse. Los argumentos son difíciles. Toman mucho poder conceptual.

Estoy completamente de acuerdo, eso tiene mucho sentido.

Problema

Sin embargo, con bastante frecuencia me encuentro creando una lista a partir de otra lista y tendré que vivir con uno de los dos males.

O uso dos líneas en el bloque , una para crear la cosa, otra para agregarla al resultado:

    public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
    {
        List<Flurp> flurps = new List<Flurp>();
        foreach (BadaBoom badaBoom in badaBooms)
        {
            Flurp flurp = CreateFlurp(badaBoom);
            flurps.Add(flurp);
        }
        return flurps;
    }

O agrego un argumento a la función para la lista donde se agregará la cosa, lo que lo convierte en "un argumento peor".

    public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
    {
        List<Flurp> flurps = new List<Flurp>();
        foreach (BadaBoom badaBoom in badaBooms)
        {
            CreateFlurpInList(badaBoom, flurps);
        }
        return flurps;
    }

Pregunta

¿Hay (des) ventajas que no estoy viendo, que hacen que una de ellas sea preferible en general? ¿O hay tales ventajas en ciertas situaciones; en ese caso, ¿qué debo buscar al tomar una decisión?

R. Schmitz
fuente
58
¿Qué tiene de malo flurps.Add(CreateFlurp(badaBoom));?
cmaster
47
No, es solo una declaración. Es solo una expresión trivialmente anidada (un solo nivel anidado). Y si un simple f(g(x))está en contra de su guía de estilo, bueno, no puedo arreglar su guía de estilo. Quiero decir, tampoco te divides sqrt(x*x + y*y)en cuatro líneas, ¿verdad? Y eso es tres (!) Subexpresiones anidadas en dos (!) Niveles de anidamiento internos (¡jadeo!). Su objetivo debe ser la legibilidad , no las declaraciones de un solo operador. Si quieres lo siguiente, bueno, tengo el lenguaje perfecto para ti: ensamblador.
cmaster
66
@cmaster Incluso el ensamblado x86 no tiene estrictamente declaraciones de un solo operador. Los modos de direccionamiento de memoria incluyen muchas operaciones complicadas y pueden usarse para la aritmética; de hecho, puede hacer una computadora completa de Turing usando solo movinstrucciones x86 y una sola jmp toStartal final. Alguien realmente hizo un compilador que hace exactamente eso: D
Luaan
55
@Luaan No hablar de las infames rlwimiinstrucciones sobre el PPC. (Eso significa Rotar inserción de máscara inmediata de palabra izquierda). Este comando tomó no menos de cinco operandos (dos registros y tres valores inmediatos), y realizó las siguientes operaciones: Un contenido de registro fue rotado por un cambio inmediato, una máscara fue creado con una sola ejecución de 1 bits que fue controlado por los otros dos operandos inmediatos, y los bits que correspondían a 1 bits en esa máscara en el otro operando de registro fueron reemplazados por los bits correspondientes del registro girado. Muy buena instrucción :-)
cmaster
77
@ R.Schmitz "Estoy haciendo programación de propósito general" - en realidad no, no lo estás haciendo, estás programando para un propósito específico (no sé para qué , pero supongo que sí ;-). Hay literalmente miles de propósitos para la programación, y los estilos de codificación óptimos para ellos varían, por lo que lo que es apropiado para usted puede no ser adecuado para otros, y viceversa: a menudo el consejo aquí es absoluto (" siempre hacer X; Y es malo "etc.) ignorando que en algunos dominios es totalmente impráctico seguir. Es por eso que los consejos en libros como Clean Code siempre deben tomarse con una pizca de sal (práctica) :)
psmears

Respuestas:

104

Estas pautas son una brújula, no un mapa. Te señalan en una dirección sensata . Pero realmente no pueden decirle en términos absolutos qué solución es la "mejor". En algún momento, debe dejar de caminar en la dirección que apunta su brújula, porque ha llegado a su destino.

Clean Code lo alienta a dividir su código en bloques muy pequeños y obvios. Esa es una buena dirección en general. Pero cuando se lleva al extremo (como sugiere una interpretación literal del consejo citado), habrá subdividido su código en partes inútiles. Nada realmente hace nada, todo solo delega. Este es esencialmente otro tipo de ofuscación de código.

Es su trabajo equilibrar “más pequeño es mejor” contra “demasiado pequeño es inútil” Pregúntese qué solución es más simple. Para mí, esa es claramente la primera solución, ya que obviamente reúne una lista. Este es un idioma bien entendido. Es posible entender ese código sin tener que mirar otra función más.

Si es posible hacerlo mejor, es al notar que "transformar todos los elementos de una lista a otra lista" es un patrón común que a menudo se puede abstraer mediante el uso de una map()operación funcional . En C #, creo que se llama Select. Algo como esto:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(BadaBoom => CreateFlurp(badaBoom)).ToList();
}
amon
fuente
77
El código sigue siendo incorrecto y reinventa la rueda sin sentido. ¿Por qué llamar CreateFlurps(someList)cuando el BCL ya proporciona someList.ConvertAll(CreateFlurp)?
Ben Voigt
44
@BenVoigt Esta es una pregunta de nivel de diseño. No me preocupa la sintaxis exacta, especialmente porque una pizarra no tiene un compilador (y la última vez que escribí C # fue en '09). Mi punto no es "he mostrado el mejor código posible", sino "como un aparte, este es un patrón común que ya está resuelto". Linq es una forma de hacerlo, ConvertAll menciona todo otro . Gracias por sugerir esa alternativa.
amon
1
Su respuesta es sensata, pero el hecho de que LINQ abstraiga la lógica y reduzca la declaración a una línea después de todo parece contradecir su consejo. Como nota al margen, BadaBoom => CreateFlurp(badaBoom)es redundante; puede pasar CreateFlurpcomo la función directamente ( Select(CreateFlurp)). (Hasta donde yo sé, este siempre ha sido el caso.)
jpmc26
2
Tenga en cuenta que esto elimina la necesidad del método por completo. El nombre CreateFlurpses en realidad más engañoso y más difícil de entender que solo ver badaBooms.Select(CreateFlurp). Este último es completamente declarativo: no hay problema para descomponerse y, por lo tanto, no se necesita ningún método.
Carl Leth
1
@ R.Schmitz No es difícil de entender, pero es menos fácil de entender que badaBooms.Select(CreateFlurp). Crea un método para que su nombre (nivel alto) represente su implementación (nivel bajo). En este caso, están al mismo nivel, así que para saber exactamente qué está sucediendo, solo tengo que mirar el método (en lugar de verlo en línea). CreateFlurps(badaBooms)podría contener sorpresas, pero badaBooms.Select(CreateFlurp)no puede. También es engañoso porque erróneamente pide un en Listlugar de un IEnumerable.
Carl Leth
61

El número ideal de argumentos para una función es cero (niládico)

¡No! El número ideal de argumentos para una función es uno. Si es cero, está garantizando que la función tiene que acceder a información externa para poder realizar una acción. "Tío" Bob se equivocó mucho.

Con respecto a su código, su primer ejemplo solo tiene dos líneas en el bloque porque está creando una variable local en la primera línea. Elimine esa asignación y está cumpliendo con estas pautas de código limpio:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    List<Flurp> flurps = new List<Flurp>();
    foreach (BadaBoom badaBoom in badaBooms)
    {
        flurps.Add(CreateFlurp(badaBoom));
    }
    return flurps;
}

Pero ese es un código muy largo (C #). Solo hazlo como:

IEnumerable<Flurp> CreateFlurps(IEnumerable<BadaBoom> badaBooms) =>
    from badaBoom in babaBooms select CreateFlurp(badaBoom);
David Arno
fuente
14
Una función con cero argumentos tiene la intención de implicar que el objeto encapsula los datos requeridos, no que las cosas existan en un estado global fuera de un objeto.
Ryathal
19
@Ryathal, dos puntos: (1) si está hablando de métodos, entonces, para la mayoría (¿todos?) De los lenguajes OO, ese objeto se infiere (o se establece explícitamente, en el caso de Python) como primer parámetro. En Java, C #, etc., todos los métodos son funciones con al menos un parámetro. El compilador solo te oculta ese detalle. (2) Nunca mencioné "global". El estado del objeto es externo a un método, por ejemplo.
David Arno
17
Estoy bastante seguro, cuando el tío Bob escribió "cero" se refería a "cero (sin contar esto)".
Doc Brown
26
@DocBrown, probablemente porque es un gran fanático de mezclar el estado y la funcionalidad en los objetos, por lo que por "función" probablemente se refiere específicamente a los métodos. Y todavía estoy en desacuerdo con él. Es mucho mejor solo darle a un método lo que necesita, en lugar de dejarlo hurgar en el objeto para obtener lo que quiere (es decir, es el clásico "decir, no preguntar" en acción).
David Arno
8
@AlessandroTeruzzi, el ideal es un parámetro. Cero es muy poco. Esta es la razón por la cual, por ejemplo, los lenguajes funcionales adoptan uno como el número de parámetros para fines de currículum (de hecho, en algunos lenguajes funcionales, todas las funciones tienen exactamente un parámetro: ni más, ni menos). El curry con cero parámetros no tendría sentido. Afirmar que "el ideal es lo menos posible, ergo cero es el mejor" es un ejemplo de reducción ad absurdum .
David Arno
19

El consejo de 'Código limpio' está completamente equivocado.

Use dos o más líneas en su bucle. Ocultar las mismas dos líneas en una función tiene sentido cuando son algunas matemáticas aleatorias que necesitan una descripción pero no hace nada cuando las líneas ya son descriptivas. 'Crear' y 'Agregar'

El segundo método que mencionas no tiene ningún sentido, ya que no estás obligado a agregar un segundo argumento para evitar las dos líneas.

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
    {
        List<Flurp> flurps = new List<Flurp>();
        foreach (BadaBoom badaBoom in badaBooms)
        {
            flurps.Add(badaBoom .CreateFlurp());
            //or
            badaBoom.AddToListAsFlurp(flurps);
            //or
            flurps.Add(new Flurp(badaBoom));
            //or
            //make flurps a member of the class
            //use linq.Select()
            //etc
        }
        return flurps;
    }

o

foreach(var flurp in ConvertToFlurps(badaBooms))...

Como han señalado otros, el consejo de que la mejor función es una sin argumentos está sesgado a OOP en el mejor de los casos y simplemente mal consejo en el peor

Ewan
fuente
¿Quizás quiera editar esta respuesta para que quede más clara? Mi pregunta era si una cosa supera a otra bajo Clean Code. Dices que todo está mal y luego pasas a describir una de las opciones que di. En este momento, parece que esta respuesta está siguiendo una agenda anti-Clean Code en lugar de tratar de responder la pregunta.
R. Schmitz
lo siento, interpreté tu pregunta como una sugerencia de que la primera era la forma "normal", pero te empujaron a la segunda. No estoy en contra del código limpio en general, pero esta cita es obviamente incorrecta
Ewan
19
@ R.Schmitz Yo mismo he leído "Código limpio" y sigo la mayoría de lo que dice ese libro. Sin embargo, en relación con que el tamaño de la función perfecta sea prácticamente una sola declaración, es simplemente incorrecto. El único efecto es que convierte el código de espagueti en código de arroz. El lector se pierde en la multitud de funciones triviales que solo producen un significado sensible cuando se ven juntas. Los seres humanos tienen una capacidad de memoria de trabajo limitada, y puede sobrecargarla con declaraciones o con funciones. Debes lograr un equilibrio entre los dos si quieres ser legible. ¡Evita los extremos!
cmaster
@cmaster La respuesta fue solo los primeros 2 párrafos cuando escribí ese comentario. Es una respuesta mucho mejor por ahora.
R. Schmitz
77
Francamente, preferí mi respuesta más corta. Hay demasiadas charlas diplomáticas en la mayoría de estas respuestas. El consejo citado es simplemente incorrecto, no es necesario especular sobre "lo que realmente significa" o dar vueltas para tratar de encontrar una buena interpretación.
Ewan
15

El segundo es definitivamente peor, ya que CreateFlurpInListacepta la lista y la modifica, lo que hace que la función no sea pura y difícil de razonar. Nada en el nombre del método sugiere que el método solo se agrega a la lista.

Y ofrezco la tercera y mejor opción:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(CreateFlurp).ToList();
}

Y, demonios, puede alinear ese método de inmediato si solo hay un lugar donde se usa, ya que la línea única es clara por sí misma, por lo que no necesita ser encapsulada por el método para darle significado.

Eufórico
fuente
No me quejaría tanto de que ese método "no es puro y difícil de razonar" (aunque cierto), sino de que es un método completamente innecesario para manejar un caso especial. ¿Qué sucede si quiero crear un Flurp independiente, un Flurp agregado a una matriz, a un diccionario, un Flurp que luego se busca en un diccionario y se elimina el Flurp correspondiente, etc.? Con el mismo argumento, el código Flurp también necesitaría todos estos métodos.
gnasher729
10

La versión de un argumento es mejor, pero no principalmente por la cantidad de argumentos.

La razón más importante por la que es mejor es que tiene un acoplamiento más bajo , lo que lo hace más útil, más fácil de razonar, más fácil de probar y menos probable que se convierta en copia + clones pegados.

Si me dan una CreateFlurp(BadaBoom), puedo usar que con cualquier tipo de recipiente de recogida: simple Flurp[], List<Flurp>, LinkedList<Flurp>, Dictionary<Key, Flurp>, y así sucesivamente. Pero con un CreateFlurpInList(BadaBoom, List<Flurp>), vuelvo mañana para pedirle CreateFlurpInBindingList(BadaBoom, BindingList<Flurp>)que mi modelo de vista pueda recibir la notificación de que la lista ha cambiado. ¡Qué asco!

Como beneficio adicional, es más probable que la firma más simple se ajuste a las API existentes. Dices que tienes un problema recurrente

con bastante frecuencia me encuentro creando una lista de otra lista

Es solo una cuestión de usar las herramientas disponibles. La versión más corta, más eficiente y mejor es:

var Flurps = badaBooms.ConvertAll(CreateFlurp);

No solo es menos código para escribir y probar, sino que también es más rápido, porque List<T>.ConvertAll()es lo suficientemente inteligente como para saber que el resultado tendrá el mismo número de elementos que la entrada, y preasigna la lista de resultados al tamaño correcto. Mientras que su código (ambas versiones) requería aumentar la lista.

Ben Voigt
fuente
No utilice List.ConvertAll. Se llama la forma idiomática de asignar un número enumerable de objetos a diferentes objetos en C # Select. La única razón por la ConvertAllque incluso está disponible aquí es porque el OP está pidiendo erróneamente un Listmétodo, debería ser un IEnumerable.
Carl Leth
6

Tenga en cuenta el objetivo general: hacer que el código sea fácil de leer y mantener.

A menudo, será posible agrupar varias líneas en una sola función significativa. Hazlo en estos casos. Ocasionalmente, deberá reconsiderar su enfoque general.

Por ejemplo, en su caso, reemplazando toda la implementación con var

flups = badaBooms.Select(bb => new Flurp(bb));

Podría ser una posibilidad. O podrías hacer algo como

flups.Add(new Flurp(badaBoom))

A veces, la solución más limpia y legible simplemente no cabe en una sola línea. Entonces tendrás dos líneas. No haga que el código sea más difícil de entender, solo para cumplir alguna regla arbitraria.

Su segundo ejemplo es (en mi opinión) considerablemente más difícil de entender que el primero. No es solo que tenga un segundo parámetro, es que la función modifica el parámetro. Mira lo que dice Clean Code sobre eso. (No tengo el libro a la mano en este momento, pero estoy bastante seguro de que básicamente es "no hagas eso si puedes evitarlo").

te doblo
fuente