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?
fuente
flurps.Add(CreateFlurp(badaBoom));
?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 dividessqrt(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.mov
instrucciones x86 y una solajmp toStart
al final. Alguien realmente hizo un compilador que hace exactamente eso: Drlwimi
instrucciones 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 :-)Respuestas:
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 llamaSelect
. Algo como esto:fuente
CreateFlurps(someList)
cuando el BCL ya proporcionasomeList.ConvertAll(CreateFlurp)
?BadaBoom => CreateFlurp(badaBoom)
es redundante; puede pasarCreateFlurp
como la función directamente (Select(CreateFlurp)
). (Hasta donde yo sé, este siempre ha sido el caso.)CreateFlurps
es en realidad más engañoso y más difícil de entender que solo verbadaBooms.Select(CreateFlurp)
. Este último es completamente declarativo: no hay problema para descomponerse y, por lo tanto, no se necesita ningún método.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, perobadaBooms.Select(CreateFlurp)
no puede. También es engañoso porque erróneamente pide un enList
lugar de unIEnumerable
.¡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:
Pero ese es un código muy largo (C #). Solo hazlo como:
fuente
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.
o
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
fuente
El segundo es definitivamente peor, ya que
CreateFlurpInList
acepta 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:
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.
fuente
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: simpleFlurp[]
,List<Flurp>
,LinkedList<Flurp>
,Dictionary<Key, Flurp>
, y así sucesivamente. Pero con unCreateFlurpInList(BadaBoom, List<Flurp>)
, vuelvo mañana para pedirleCreateFlurpInBindingList(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
Es solo una cuestión de usar las herramientas disponibles. La versión más corta, más eficiente y mejor es:
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.fuente
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 laConvertAll
que incluso está disponible aquí es porque el OP está pidiendo erróneamente unList
método, debería ser unIEnumerable
.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
Podría ser una posibilidad. O podrías hacer algo como
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").
fuente