Mi jefe me pide que deje de escribir funciones pequeñas y que haga todo en el mismo ciclo

209

He leído un libro llamado Clean Code de Robert C. Martin. En este libro he visto muchos métodos para limpiar código, como escribir pequeñas funciones, elegir nombres cuidadosamente, etc. Parece, con mucho, el libro más interesante sobre código limpio que he leído. Sin embargo, hoy a mi jefe no le gustó la forma en que escribí el código después de leer este libro.

Sus argumentos fueron

  • Escribir pequeñas funciones es una molestia porque te obliga a pasar a cada pequeña función para ver qué está haciendo el código.
  • Ponga todo en un bucle principal principal, incluso si el bucle principal tiene más de 300 líneas, es más rápido de leer.
  • Solo escriba funciones pequeñas si tiene que duplicar el código.
  • No escriba una función con el nombre del comentario, coloque su línea de código compleja (3-4 líneas) con un comentario arriba; Del mismo modo, puede modificar el código que falla directamente

Esto va en contra de todo lo que he leído. ¿Cómo sueles escribir código? ¿Un gran bucle principal, sin funciones pequeñas?

El lenguaje que uso es principalmente Javascript. Realmente tengo dificultades para leer ahora, ya que he eliminado todas mis pequeñas funciones claramente nombradas y he puesto todo en un gran bucle. Sin embargo, a mi jefe le gusta de esta manera.

Un ejemplo fue:

// The way I would write it
if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

// The way he would write it
// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

En el libro que he leído, por ejemplo, los comentarios se consideran una falla al escribir código limpio porque son obsoletos si escribes funciones pequeñas y a menudo conducen a comentarios no actualizados (modificas tu código y no el comentario). Sin embargo, lo que hago es eliminar el comentario y escribir una función con el nombre del comentario.

Bueno, me gustaría un consejo, ¿qué forma / práctica es mejor escribir código limpio?

GitCommit Victor B.
fuente
44
phoneNumber = headers.resourceId?: DEV_PHONE_NUMBER;
Joshua
10
Valide que desea trabajar en el lugar, donde la gerencia le dijo CÓMO hacer su trabajo, en lugar de QUÉ debe resolverse.
Konstantin Petrukhnov
8
@rjmunro A menos que realmente le guste su trabajo, tenga en cuenta que hay menos desarrolladores que trabajos. Entonces, para citar a Martin Fowler: "¡Si no puede cambiar su organización, cambie su organización!" y los jefes que me dicen cómo codificar es algo que les aconsejo que quieran cambiar.
Niels van Reijmersdal
10
¡NUNCA tenga una isApplicationInProduction()función! Debe tener pruebas, y las pruebas son inútiles si su código se comporta de manera diferente a cuando es producción. Es como tener intencionalmente código no probado / descubierto en producción: no tiene sentido.
Ronan Paixão

Respuestas:

215

Tomando los ejemplos de código primero. Usted favorece:

if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Y tu jefe lo escribiría como:

// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

En mi opinión, ambos tienen problemas. Mientras leía su código, mi pensamiento inmediato fue "puede reemplazar eso ifcon una expresión ternaria". Luego leí el código de su jefe y pensé "¿por qué reemplazó su función con un comentario?".

Sugeriría que el código óptimo está entre los dos:

phoneNumber = isApplicationInProduction(headers) ? headers.resourceId : DEV_PHONE_NUMBER;

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Eso le brinda lo mejor de ambos mundos: una expresión de prueba simplificada y el comentario se reemplaza con código comprobable.

Sin embargo, con respecto a las opiniones de su jefe sobre el diseño del código:

Escribir pequeñas funciones es una molestia porque te obliga a pasar a cada una de las pequeñas funciones para ver qué está haciendo el código.

Si la función está bien nombrada, este no es el caso. isApplicationInProductiones evidente y no debería ser necesario examinar el código para ver qué hace. De hecho, lo contrario es cierto: examinar el código revela menos en cuanto a la intención que el nombre de la función (es por eso que su jefe tiene que recurrir a los comentarios).

Ponga todo en un bucle principal principal, incluso si el bucle principal tiene más de 300 líneas, es más rápido de leer

Puede ser más rápido escanear, pero para realmente "leer" el código, debe ser capaz de ejecutarlo efectivamente en su cabeza. Eso es fácil con funciones pequeñas y es muy, muy difícil con métodos que tienen cientos de líneas de largo.

Escriba solo funciones pequeñas si tiene que duplicar código

Estoy en desacuerdo. Como muestra su ejemplo de código, las funciones pequeñas y bien nombradas mejoran la legibilidad del código y deben usarse siempre que, por ejemplo, no le interese el "cómo", solo el "qué" de una funcionalidad.

No escriba una función con el nombre del comentario, coloque su línea de código compleja (3-4 líneas) con un comentario arriba. De esta manera, puede modificar el código que falla directamente

Realmente no puedo entender el razonamiento detrás de este, suponiendo que realmente sea serio. Es el tipo de cosas que esperaría ver escritas en parodia por la cuenta de Twitter de The Expert Beginner . Los comentarios tienen una falla fundamental: no se compilan / interpretan y, por lo tanto, no pueden ser probados en unidades. El código se modifica y el comentario se queda solo y terminas sin saber cuál es el correcto.

Escribir código autodocumentado es difícil, y a veces se necesitan documentos complementarios (incluso en forma de comentarios). Pero la opinión del "Tío Bob" de que los comentarios son una falla de codificación es cierta con demasiada frecuencia.

Haga que su jefe lea el libro Clean Code e intente resistirse a hacer que su código sea menos legible solo para satisfacerlo. Sin embargo, en última instancia, si no puede persuadirlo para que cambie, debe alinearse o buscar un nuevo jefe que pueda codificar mejor.

David Arno
fuente
26
Pequeñas funciones se prueban
fácilmente en la
13
Quoth @ ExpertBeginner1 : "Estoy cansado de ver toneladas de pequeños métodos en todo el lugar en nuestro código, así que de aquí en adelante, hay un mínimo de 15 LOC en todos los métodos".
Greg Bacon
34
"Los comentarios tienen un defecto fundamental: no se compilan / interpretan y, por lo tanto, no se pueden probar de forma unitaria". En este caso, haciendo de abogado del diablo, esto también es cierto si reemplaza "comentarios" con "nombres de funciones".
mattecapu
11
@mattecapu, tomaré tu defensa y la duplicaré de inmediato. Cualquier desarrollador de basura viejo puede exagerar en un comentario tratando de describir lo que hace un fragmento de código. La descripción sucinta de ese fragmento de código con un buen nombre de función requiere un comunicador experto. Los mejores desarrolladores son comunicadores expertos, ya que escribir código se ocupa principalmente de comunicarse con otros desarrolladores y del compilador como una preocupación secundaria. Ergo, un buen desarrollador usará funciones bien nombradas y no hará comentarios; los desarrolladores pobres ocultan sus habilidades pobres detrás de excusas para usar comentarios.
David Arno
44
@DavidArno Todas las funciones tienen condiciones previas y posteriores, la pregunta es si las documenta o no. Si mi función toma un parámetro, que es una distancia en pies medidos, debe proporcionarlo en pies, no en kilómetros. Esta es una condición previa.
Jørgen Fogh
223

Hay otros problemas

Ninguno de los códigos es bueno, porque ambos básicamente hinchan el código con un caso de prueba de depuración . ¿Qué pasa si quieres probar más cosas por cualquier razón?

phoneNumber = DEV_PHONE_NUMBER_WHICH_CAUSED_PROBLEMS_FOR_CUSTOMERS;

o

phoneNumber = DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY;

¿Quieres agregar aún más ramas?

El problema importante es que básicamente duplica una parte de su código y, por lo tanto, en realidad no está probando el código real. Escribe código de depuración para probar el código de depuración, pero no el código de producción. Es como crear parcialmente una base de código paralela.

Usted está discutiendo con su jefe sobre cómo escribir códigos incorrectos de manera más inteligente. En cambio, debe solucionar el problema inherente del código en sí.

Inyección de dependencia

Así es como debería verse su código:

phoneNumber = headers.resourceId;

No hay ramificación aquí, porque la lógica aquí no tiene ninguna ramificación. El programa debe extraer el número de teléfono del encabezado. Período.

Si desea tener DEV_PHONE_NUMBER_FROM_OTHER_COUNTRYcomo resultado, debe ponerlo en headers.resourceId. Una forma de hacerlo es simplemente inyectar un headersobjeto diferente para casos de prueba (perdón si este no es el código apropiado, mis habilidades de JavaScript están un poco oxidadas):

function foo(headers){
    phoneNumber = headers.resourceId;
}

// Creating the test case
foo({resourceId: DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY});

Suponiendo que eso headerssea ​​parte de una respuesta que reciba de un servidor: lo ideal sería tener un servidor de prueba completo que ofrezca headersvarios tipos de prueba. De esta manera, prueba el código de producción real tal cual y no un código medio duplicado que puede o no funcionar como el código de producción.

nulo
fuente
11
Pensé en abordar este mismo tema en mi propia respuesta, pero sentí que ya era lo suficientemente largo. Entonces +1 para ti por hacerlo :)
David Arno
55
@DavidArno Estaba a punto de agregarlo como un comentario a su respuesta, porque la pregunta todavía estaba bloqueada cuando la leí por primera vez, para mi sorpresa, estaba abierta nuevamente y agregué esto como respuesta. Tal vez debería agregarse que hay docenas de marcos / herramientas para realizar pruebas automatizadas. Especialmente en JS parece que sale uno nuevo todos los días. Sin embargo, podría ser difícil venderle eso al jefe.
nulo
56
@DavidArno Tal vez deberías haber dividido tu respuesta en respuestas más pequeñas. ;)
krillgar
2
@ user949300 Usar un OR bit a bit no sería inteligente;)
curiousdannii
1
@curiousdannii Sí, noté que es demasiado tarde para editar ...
user949300
59

No hay una respuesta "correcta" o "incorrecta" a esto. Sin embargo, ofreceré mi opinión basada en 36 años de experiencia profesional diseñando y desarrollando sistemas de software ...

  1. No existe el "código autodocumentado". ¿Por qué? Porque ese reclamo es completamente subjetivo.
  2. Los comentarios nunca son fallas. Lo que es una falla es un código que no puede entenderse sin comentarios.
  3. 300 líneas de código ininterrumpidas en un bloque de código es una pesadilla de mantenimiento y muy propenso a errores. Tal bloque es fuertemente indicativo de un mal diseño y planificación.

Hablando directamente con el ejemplo que proporcionó ... Colocarse isApplicationInProduction()en su propia rutina es lo más inteligente. Hoy esa prueba es simplemente una verificación de "encabezados" y puede manejarse en un ?:operador ternario ( ). Mañana, la prueba puede ser mucho más compleja. Además, "headers.resourceId" no tiene una relación clara con el "estado de producción" de la aplicación; Yo diría que una prueba para tal estado necesita ser desacoplada de los datos subyacentes; una subrutina hará esto y un ternario no. Además, un comentario útil sería por qué resourceId es una prueba para "en producción".

Tenga cuidado de no exagerar con "pequeñas funciones claramente nombradas". Una rutina debería encapsular una idea más que "solo código". Apoyo la sugerencia de amon phoneNumber = getPhoneNumber(headers)y agrego que getPhoneNumber()debería hacer la prueba de "estado de producción" conisApplicationInProduction()

LiamF
fuente
25
Hay buenos comentarios que no son un fracaso. Sin embargo, los comentarios que son casi literalmente el código que supuestamente explican o son solo bloques de comentarios vacíos que preceden a un método / clase / etc. son definitivamente un fracaso
jpmc26
3
Es posible tener un código que sea más pequeño y más fácil de leer que cualquier descripción en inglés de lo que hace y los casos de esquina que maneja y no maneja. Además, si una función se despliega en su propio método, alguien que lea la función no sabrá qué casos de esquina son o no manejados por sus llamantes, y a menos que el nombre de la función sea muy detallado, alguien que examine a las personas que llaman puede no saber qué esquina los casos son manejados por la función.
supercat
77
Los comentarios nunca son intrínsecamente fallidos. Los comentarios pueden ser fallidos, y lo son cuando son inexactos. El código incorrecto se puede detectar en varios niveles, incluso por un comportamiento incorrecto en el modo de recuadro negro. Los comentarios incorrectos solo pueden detectarse mediante la comprensión humana en el modo de recuadro blanco, mediante el reconocimiento de que se describen dos modelos y que uno de ellos es incorrecto.
Timbo
77
@Timbo Quieres decir, "... al menos uno de ellos es incorrecto". ;)
jpmc26
55
@immibis Si no puede entender lo que hace el código sin comentarios, entonces el código probablemente no sea lo suficientemente claro. El objetivo principal de los comentarios es aclarar por qué el código está haciendo lo que está haciendo. Es el codificador que explica su diseño a los futuros mantenedores. El código nunca puede proporcionar ese tipo de explicación, por lo que los comentarios llenan esos vacíos en la comprensión.
Graham
47

"Las entidades no deben multiplicarse más allá de la necesidad".

- La navaja de Occam

El código debe ser lo más simple posible. A los errores les gusta esconderse entre la complejidad, porque son difíciles de detectar allí. Entonces, ¿qué hace que el código sea simple?

Las unidades pequeñas (archivos, funciones, clases) son una buena idea . Las unidades pequeñas son fáciles de entender porque hay menos cosas que tienes que entender a la vez. Los humanos normales solo pueden hacer malabarismos con ~ 7 conceptos a la vez. Pero el tamaño no solo se mide en líneas de código . Puedo escribir la menor cantidad de código posible "jugando golf" al código (eligiendo nombres de variables cortos, tomando atajos "inteligentes", rompiendo la mayor cantidad de código posible en una sola línea), pero el resultado final no es simple. Intentar entender dicho código es más como ingeniería inversa que como lectura.

Una forma de acortar una función es extraer varias funciones auxiliares. Esa puede ser una buena idea cuando extrae una pieza de complejidad autónoma . En forma aislada, esa parte de complejidad es mucho más simple de administrar (¡y probar!) Que cuando se incrusta en un problema no relacionado.

Pero cada llamada de función tiene una sobrecarga cognitiva : no solo tengo que entender el código dentro de mi código actual, también tengo que entender cómo interactúa con el código en el exterior . Creo que es justo decir que la función que extrajo introduce más complejidad en la función que la que extrae . Eso es lo que su jefe quiso decir con " pequeñas funciones [son] un dolor porque lo obliga a pasar a cada pequeña función para ver qué está haciendo el código. "

A veces, las funciones largas y aburridas pueden ser increíblemente simples de entender, incluso cuando tienen cientos de líneas de largo. Esto tiende a suceder en el código de inicialización y configuración, por ejemplo, al crear una GUI a mano sin un editor de arrastrar y soltar. No hay una pieza de complejidad autónoma que pueda extraer razonablemente. Pero si el formato es legible y hay algunos comentarios, realmente no es difícil seguir lo que está sucediendo.

Hay muchas otras métricas de complejidad: el número de variables en un ámbito debe ser lo más pequeño posible. Eso no significa que debamos evitar las variables. Significa que debemos restringir cada variable al alcance más pequeño posible donde sea necesario. Las variables también se vuelven más simples si nunca cambiamos el valor que contienen.

Una métrica muy importante es la complejidad ciclomática (complejidad de McCabe). Mide el número de rutas independientes a través de un fragmento de código. Este número crece exponencialmente con cada condicional. Cada condicional o bucle duplica el número de rutas. Hay evidencia que sugiere que un puntaje de más de 10 puntos es demasiado complejo. Esto significa que una función muy larga que tal vez tenga un puntaje de 5 es quizás mejor que una función muy corta y densa con un puntaje de 25. Podemos reducir la complejidad extrayendo el flujo de control en funciones separadas.

Su condicional es un ejemplo de una pieza de complejidad que podría extraerse por completo:

function bigFatFunction(...) {
  ...
  phoneNumber = getPhoneNumber(headers);
  ...
}

...

function getPhoneNumber(headers) {
  return headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;
}

Esto todavía está a punto de ser útil. No estoy seguro de si eso disminuye sustancialmente la complejidad porque este condicional no es muy condicional . En producción, siempre tomará el mismo camino.


La complejidad nunca puede desaparecer. Solo se puede barajar. ¿Son muchas cosas pequeñas más simples que pocas cosas grandes? Eso depende mucho de las circunstancias. Por lo general, hay una combinación que se siente bien. Encontrar ese compromiso entre diferentes factores de complejidad requiere intuición y experiencia, y un poco de suerte.

Saber cómo escribir funciones muy pequeñas y funciones muy simples es una habilidad útil, porque no puedes elegir sin conocer las alternativas. Seguir ciegamente las reglas o las mejores prácticas sin pensar en cómo se aplican a la situación actual conduce a resultados promedio en el mejor de los casos, programación de culto a la carga en el peor .

Ahí es donde no estoy de acuerdo con tu jefe. Sus argumentos no son inválidos, pero tampoco el libro Clean Code está equivocado. Probablemente sea mejor seguir las pautas de su jefe, pero el hecho de que esté pensando en estos problemas, tratando de encontrar una mejor manera, es muy prometedor. A medida que gane experiencia, le resultará más fácil encontrar un buen factoring para su código.

(Nota: esta respuesta se basa en partes en los pensamientos de la publicación del blog Reasonable Code en The Whiteboard por Jimmy Hoffa , que proporciona una vista de alto nivel sobre lo que hace que el código sea simple).

amon
fuente
Soy general, me gustó tu respuesta. Sí, sin embargo, estoy en desacuerdo con la medida de complejidad ciclomática mcabes. Por lo que he visto de él, no presenta una verdadera medida de complejidad.
Robert Baron
27

El estilo de programación de Robert Martin es polarizante. Encontrará muchos programadores, incluso experimentados, que encuentran muchas excusas por las que dividir "tanto" es demasiado, y por qué mantener las funciones un poco más grandes es "la mejor manera". Sin embargo, la mayoría de estos "argumentos" son a menudo una expresión de falta de voluntad para cambiar los viejos hábitos y aprender algo nuevo.

¡No los escuches!

Siempre que pueda guardar un comentario refactorizando un fragmento de código a una función separada con un nombre expresivo, hágalo; lo más probable es que mejore su código. No tiene que ir tan lejos como lo hace Bob Martin en su libro de códigos limpio, pero la gran mayoría del código que he visto en el pasado que causó problemas de mantenimiento contenía funciones demasiado grandes, no demasiado pequeñas. Intentar escribir funciones más pequeñas con nombres autodescriptivos es lo que debe intentar.

Las herramientas de refactorización automáticas hacen que extraer métodos sea fácil, simple y seguro. Y, por favor, no tome en serio a las personas que recomiendan escribir funciones con más de 300 líneas; estas personas definitivamente no están calificadas para decirle cómo debe codificar.

Doc Brown
fuente
53
"¡No los escuches!" : dado que su jefe le pide al OP que deje de dividir el código, el OP probablemente debería evitar su consejo. Incluso si el jefe no está dispuesto a cambiar sus viejos hábitos. También tenga en cuenta que, como lo resaltaron las respuestas anteriores, tanto el código del OP como el código de su jefe están mal escritos, y usted (intencionalmente o no) no menciona eso en su respuesta.
Arseni Mourzenko
10
@ArseniMourzenko: no todos tenemos que ceder ante su jefe. Espero que el OP tenga la edad suficiente para saber cuándo tiene que hacer lo correcto o cuándo tiene que hacer lo que su jefe le dice. Y sí, no estaba entrando en los detalles del ejemplo intencionalmente, ya hay suficientes otras respuestas discutiendo esos detalles.
Doc Brown
8
@DocBrown De acuerdo. 300 líneas es cuestionable para toda una clase. Una función de 300 líneas es obscena.
JimmyJames
30
He visto muchas clases que tienen más de 300 líneas de largo que son clases perfectamente buenas. Java es tan detallado que casi no puedes hacer nada significativo en una clase sin tanto código. Entonces, "número de líneas de código en una clase", en sí mismo, no es una métrica significativa, más de lo que consideraríamos SLOC como una métrica significativa para la productividad del programador.
Robert Harvey
99
Además, he visto los sabios consejos del tío Bob malinterpretados y abusados ​​tanto que tengo mis dudas de que sea útil para cualquiera que no sea un programador experimentado .
Robert Harvey
23

En su caso: desea un número de teléfono. O es obvio cómo obtendrías un número de teléfono, luego escribes el código obvio. O no es obvio cómo obtendría un número de teléfono, luego escriba un método para ello.

En su caso, no es obvio cómo obtener el número de teléfono, por lo que debe escribir un método para ello. La implementación no es obvia, pero es por eso que la está colocando en un método separado, por lo que debe tratarla solo una vez. Un comentario sería útil porque la implementación no es obvia.

El método "isApplicationInProduction" no tiene sentido. Llamarlo desde su método getPhonenumber no hace que la implementación sea más obvia, y solo hace que sea más difícil descubrir qué está sucediendo.

No escribas funciones pequeñas. Escriba funciones que tengan un propósito bien definido y cumplan con ese propósito bien definido.

PD. No me gusta la implementación en absoluto. Se supone que la ausencia de un número de teléfono significa que es una versión de desarrollo. Entonces, si el número de teléfono está ausente en la producción, no solo no lo maneja, sino que lo sustituye por un número de teléfono aleatorio. Imagine que tiene 10,000 clientes, y 17 no tiene un número de teléfono, y tiene problemas en la producción. Ya sea que esté en producción o desarrollo debe verificarse directamente, no derivado de otra cosa.

gnasher729
fuente
1
"No escriba funciones pequeñas. Escriba funciones que tengan un propósito bien definido y cumplan ese propósito bien definido". ese es el criterio correcto para dividir el código. Si una función tiene demasiadas (como más de una) funciones dispares , divídala. El principio de responsabilidad única es el principio rector.
Robert Bristow-Johnson el
16

Incluso ignorando el hecho de que ninguna de las implementaciones es tan buena, me gustaría señalar que esto es esencialmente una cuestión de gusto al menos a nivel de abstracción de funciones triviales de un solo uso.

El número de líneas no es una métrica útil en la mayoría de los casos.

300 (o incluso 3000) líneas de código completamente trivial puramente secuencial (Configuración, o algo así) rara vez es un problema (pero podría generarse mejor automáticamente o como una tabla de datos o algo así), 100 líneas de bucles anidados con muchos complicados las condiciones de salida y las matemáticas, como puede encontrar en Eliminación gaussiana o inversión de matriz o tal, pueden ser demasiado para seguir fácilmente.

Para mí, no escribiría una función de un solo uso a menos que la cantidad de código requerida para declarar la cosa fuera mucho menor que la cantidad de código que forma la implementación (a menos que tenga razones como decir querer poder realizar fácilmente la inyección de fallas). Un condicional único rara vez se ajusta a este proyecto de ley.

Ahora vengo de un pequeño mundo incrustado, donde también tenemos que considerar cosas como la profundidad de la pila y los gastos generales de llamada / retorno (que nuevamente argumenta contra el tipo de funciones pequeñas que parecen recomendarse aquí), y esto podría estar sesgando mi diseño decisiones, pero si vi esa función original en una revisión de código, obtendría una llama de usenet de estilo antiguo en respuesta.

El gusto es que el diseño es difícil de enseñar y solo viene con experiencia, no estoy seguro de que pueda reducirse a reglas sobre longitudes de funciones, e incluso la complejidad ciclomática tiene sus límites como una métrica (a veces las cosas son complicadas, sin embargo, las abordas).
Esto no quiere decir que el código limpio no discuta algunas cosas buenas, sí lo hace, y estas cosas deberían pensarse en las costumbres locales y en lo que hace la base de código existente también se debe dar peso.

Este ejemplo en particular me parece que estoy escogiendo con detalles triviales, estaría más preocupado por cosas de nivel mucho más alto, ya que eso es mucho más importante para la capacidad de comprender y depurar un sistema fácilmente.

Dan Mills
fuente
1
Estoy totalmente de acuerdo: tomaría una línea compleja muy compleja para mí considerar envolverlo en una función ... Ciertamente no envolvería una línea de valor ternario / predeterminado. He envuelto un revestimiento, pero generalmente ese es el script de shell donde hay diez tuberías para analizar algo y el código es incomprensible sin ejecutarlo.
TemporalWolf
15

No ponga todo en un gran bucle, pero tampoco haga esto con demasiada frecuencia:

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

El problema con el bucle grande es que es realmente difícil ver su estructura general cuando abarca muchas pantallas. Por lo tanto, trate de sacar trozos grandes, idealmente trozos que tengan una única responsabilidad y sean reutilizables.

El problema con la pequeña función anterior es que, si bien la atomicidad y la modularidad son generalmente buenas, eso puede llevarse demasiado lejos. Si no va a reutilizar la función anterior, esto disminuye la legibilidad y facilidad de mantenimiento del código. Para profundizar en los detalles, debe saltar a la función en lugar de poder leer los detalles en línea, y la llamada a la función ocupa apenas menos espacio que el detalle en sí.

Claramente, se puede encontrar un equilibrio entre los métodos que hacen demasiado y los métodos que hacen muy poco . Nunca revelaría una pequeña función como la anterior, a menos que se llamara desde más de un lugar, e incluso entonces lo pensaría dos veces, porque la función no es tan importante en términos de introducir una nueva lógica y apenas justifica tener su propia existencia.

Brad Thomas
fuente
2
Entiendo que un solo booleano es fácil de leer, pero eso solo solo explica "Qué" está sucediendo. Todavía escribo funciones que envuelven expresiones ternarias simples porque el nombre de la función ayuda a explicar el motivo "Por qué" estoy realizando esta comprobación de condición. Esto es especialmente útil cuando alguien nuevo (o usted mismo en 6 meses) necesita comprender la lógica del negocio.
AJ X.
14

Parece que lo que realmente quieres es esto:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER

Esto debe explicarse por sí mismo a cualquiera que lo lea: fijar phoneNumbera la resourceIdsi está disponible, o por defecto a DEV_PHONE_NUMBERsi no lo es.

Si realmente desea establecer esa variable solo en producción, debería tener algún otro método de utilidad más canónico para toda la aplicación (que no requiera parámetros) para determinar desde dónde se está ejecutando. Leer los encabezados de esa información no tiene sentido.

BlueRaja - Danny Pflughoeft
fuente
Explica por sí mismo lo que hace (con un poco de adivinación del lenguaje que estás usando), pero no es del todo obvio lo que está sucediendo. Aparentemente, el desarrollador asume que el número de teléfono está almacenado bajo "resourceId" en la versión de producción, y que resourceId no está presente en la versión de desarrollo, y quiere usar DEV_PHONE_NUMBER en la versión de desarrollo. Lo que significa que el número de teléfono está almacenado bajo un extraño título, y significa que las cosas saldrán mal si falta un número de teléfono en la versión de producción
gnasher729
14

Permítanme ser franco: me parece que su entorno (lenguaje / marco / diseño de clase, etc.) no es realmente adecuado para el código "limpio". Está mezclando todo tipo de cosas posibles en unas pocas líneas de código que realmente no deberían estar muy juntas. ¿Qué negocio tiene una sola función al saber que eso resourceId==undefsignifica que no está en producción, que está usando un número de teléfono predeterminado en sistemas que no son de producción, que el resourceId se guarda en algunos "encabezados" y así sucesivamente? Supongo que headersson encabezados HTTP, por lo que incluso deja la decisión sobre el entorno en el que se encuentra al usuario final.

Factorizar piezas individuales de eso en funciones no lo ayudará mucho con ese problema subyacente.

Algunas palabras clave a buscar:

  • desacoplamiento
  • cohesión
  • inyección de dependencia

Puede lograr lo que desea (en otros contextos) con cero líneas de código, cambiando las responsabilidades del código y utilizando marcos modernos (que pueden existir o no para su entorno / lenguaje de programación).

Según su descripción ("300 líneas de código en una función 'principal'"), incluso la palabra "función" (en lugar de método) me hace suponer que no tiene sentido lo que está tratando de lograr. En ese entorno de programación de la vieja escuela (es decir, programación básica imperativa con poca estructura, ciertamente sin clases significativas, sin un patrón de marco de clase como MVC o somesuch), realmente no tiene mucho sentido hacer nada . Nunca saldrás del sumidero sin cambios fundamentales. Al menos su jefe parece permitirle crear funciones para evitar la duplicación de código, ¡ese es un buen primer paso!

Sé tanto el tipo de código como el tipo de programador que estás describiendo bastante bien. Francamente, si fuera un compañero de trabajo, mi consejo sería diferente. Pero como es tu jefe, es inútil que pelees por esto. No solo que su jefe puede anularlo, sino que las adiciones de su código conducirán a un código peor si solo hace "lo suyo" parcialmente, y su jefe (y probablemente otras personas) continúan como antes. Puede que sea mejor para el resultado final si se adapta a su estilo de programación (solo mientras trabaja en esta base de código en particular, por supuesto) e intenta, en este contexto, sacar el máximo provecho de ella.

AnoE
fuente
1
Estoy 100% de acuerdo con que hay componentes implícitos aquí que deberían separarse, pero sin saber más sobre el lenguaje / marco, es difícil saber si un enfoque OO tiene sentido. El desacoplamiento y el principio de responsabilidad única son importantes en cualquier lenguaje, desde funcional puro (por ejemplo, Haskell) hasta imperativo puro (por ejemplo, C.) Mi primer paso, si el jefe lo permitiera, sería convertir la función principal en una función de despachador ( como un esquema o tabla de contenido) que se leería en un estilo declarativo (que describe políticas, no algoritmos) y agilizaría el trabajo a otras funciones.
David Leppik
JavaScript es prototípico, con funciones de primera clase. Es inherentemente OO, pero no en el sentido clásico, por lo que sus suposiciones pueden no ser correctas. Cue horas de ver videos de Crockford en YouTube ...
Kevin_Kinsey
13

"Limpio" es un objetivo en la escritura de código. No es el único objetivo. Otro objetivo digno es la colocalidad . Dicho de manera informal, la colocalidad significa que las personas que intentan entender su código no necesitan saltar por todas partes para ver lo que están haciendo. Usar una función bien nombrada en lugar de una expresión ternaria puede parecer algo bueno, pero dependiendo de cuántas funciones tenga y dónde se encuentren, esta práctica puede convertirse en una molestia. No puedo decirte si has cruzado esa línea, excepto para decir que si las personas se quejan, debes escuchar, especialmente si esas personas tienen algo que decir sobre tu situación laboral.

usuario1172763
fuente
2
"... excepto para decir que si la gente se queja, debe escuchar, especialmente si esas personas tienen algo que decir sobre su situación laboral". OMI, este es realmente un mal consejo. A menos que sea un desarrollador realmente pobre que necesite apreciar cualquier trabajo que pueda obtener, aplique siempre el principio "si no puede cambiar su trabajo, cambie su trabajo". Nunca se sienta obligado a una empresa; te necesitan más de lo que tú los necesitas, así que vete a un lugar mejor si no te ofrecen lo que quieres.
David Arno
44
Me he movido un poco durante mi carrera. No creo que haya tenido un trabajo en el que me haya visto 100% cara a cara con mi jefe sobre cómo codificar. Somos seres humanos con nuestros propios antecedentes y filosofías. Así que personalmente no dejaría un trabajo solo porque había algunos estándares de codificación que no me gustaban. (Las convenciones de nomenclatura con los dedos parecen gustarles a los gerentes son particularmente contrarias a la forma en que codificaría si lo dejara en mis propios dispositivos). Pero tienes razón, un programador decente no debería preocuparse demasiado por simplemente permanecer empleado .
user1172763
6

El uso de funciones pequeñas es, en general, una buena práctica. Pero, idealmente, creo que la introducción de una función debería separar grandes fragmentos lógicos o reducir el tamaño general del código SECANDO. El ejemplo que dio a ambos hace que el código sea más largo y requiere más tiempo para que un desarrollador lo lea, mientras que la alternativa breve no explica que el "resourceId"valor solo esté presente en la producción. Algo simple como eso es fácil de olvidar y confuso al tratar de trabajar con él, especialmente si aún eres nuevo en la base de código.

No diré que debería usar absolutamente un ternario, algunas personas con las que he trabajado prefieren un poco más if () {...} else {...}, es sobre todo una elección personal. Tiendo a preferir un enfoque de "una línea hace una cosa", pero básicamente me limitaré a lo que la base de código normalmente usa.

Cuando se usa ternary si la verificación lógica hace que la línea sea demasiado larga o complicada, considere crear una / s variable / s bien nombrada para mantener el valor.

// NOTE "resourceId" not present in dev build, use test data
let isProduction = 'resourceId' in headers;
let phoneNumber = isProduction ? headers.resourceId : DEV_PHONE_NUMBER;

También me gustaría decir que si la base de código se extiende hacia 300 funciones de línea, entonces necesita alguna subdivisión. Pero yo recomendaría el uso de trazos ligeramente más amplios.

nepeo
fuente
5

El ejemplo de código que diste, tu jefe ES CORRECTO. Una sola línea clara es mejor en ese caso.

En general, dividir la lógica compleja en partes más pequeñas es mejor para la legibilidad, el mantenimiento del código y la posibilidad de que las subclases tengan un comportamiento diferente (aunque solo sea un poco).

No ignore las desventajas: gastos generales de la función, oscurecimiento (la función no hace lo que implican los comentarios y el nombre de la función), lógica compleja de espagueti, el potencial de funciones muertas (en un momento se crearon para un propósito que ya no se llama).

Phil M
fuente
1
"gastos generales de la función": eso depende del compilador. "oscurecimiento": OP no ha indicado si es la única o la mejor manera de verificar esa propiedad; tampoco puedes estar seguro. "lógica compleja de espagueti": ¿dónde? "el potencial de funciones inactivas": ese tipo de análisis de código inactivo es fruto de bajo nivel, y las cadenas de herramientas de desarrollo que carecen de él son inmaduras.
Rhymoid el
Las respuestas se centraron más en las ventajas, solo quería señalar desventajas también. Llamar a una función como sum (a, b) siempre será más costoso que "a + b" (a menos que el compilador incorpore la función). El resto de las desventajas demuestran que la excesiva complejidad puede conducir a su propio conjunto de problemas. El código incorrecto es un código incorrecto, y solo porque se divide en bytes más pequeños (o se mantiene en un bucle de 300 líneas) no significa que sea más fácil de tragar.
Phil M
2

Puedo pensar en al menos dos argumentos a favor de las funciones largas:

  • Significa que tiene mucho contexto alrededor de cada línea. Una forma de formalizar esto: dibuje el gráfico de flujo de control de su código. En un vértice (~ = línea) entre la entrada de la función y la salida de la función, conoce todos los bordes entrantes. Cuanto más larga sea la función, más vértices hay.

  • Muchas funciones pequeñas significan que hay un gráfico de llamadas más grande y complejo. Elija una línea aleatoria en una función aleatoria y responda la pregunta "¿en qué contexto (s) se ejecuta esta línea?" Esto se vuelve más difícil cuanto más grande y complejo es el gráfico de llamadas, porque tienes que mirar más vértices en ese gráfico.

También hay argumentos en contra de las funciones largas: la capacidad de prueba unitaria me viene a la mente. Use t̶h̶e̶ ̶f̶o̶r̶c̶e̶ su experiencia al elegir entre uno y el otro.

Nota: No estoy diciendo que tu jefe tenga razón, solo que su perspectiva puede no estar completamente desprovista de valor.


Creo que mi opinión es que el buen parámetro de optimización no es la longitud de la función. Creo que una idea más útil para pensar en términos es la siguiente: si todo lo demás es igual, es preferible poder leer en el código una descripción de alto nivel tanto de la lógica empresarial como de la implementación. (Los detalles de implementación de bajo nivel siempre se pueden leer si puede encontrar el bit de código relevante).


Comentando la respuesta de David Arno :

Escribir pequeñas funciones es una molestia porque te obliga a pasar a cada una de las pequeñas funciones para ver qué está haciendo el código.

Si la función está bien nombrada, este no es el caso. isApplicationInProduction es evidente y no debería ser necesario examinar el código para ver qué hace. De hecho, lo contrario es cierto: examinar el código revela menos en cuanto a la intención que el nombre de la función (por lo que su jefe tiene que recurrir a los comentarios).

El nombre hace evidente lo que el valor de retorno significa , pero no dice nada acerca de los efectos de la ejecución del código (= lo que el código hace ). Los nombres (solo) transmiten información sobre la intención , el código transmite información sobre el comportamiento (a partir del cual a veces se pueden inferir partes de la intención).

A veces quieres una, a veces la otra, por lo que esta observación no crea una regla de decisión universalmente válida unilateral.

Ponga todo en un bucle principal principal, incluso si el bucle principal tiene más de 300 líneas, es más rápido de leer

Puede ser más rápido escanear, pero para realmente "leer" el código, debe ser capaz de ejecutarlo efectivamente en su cabeza. Eso es fácil con funciones pequeñas y es muy, muy difícil con métodos que tienen cientos de líneas de largo.

Estoy de acuerdo en que tienes que ejecutarlo en tu cabeza. Si tiene 500 líneas de funcionalidad en una función grande frente a muchas funciones pequeñas, no me queda claro por qué esto se hace más fácil.

Suponga el caso extremo de 500 líneas de código de efectos secundarios de línea recta y desea saber si el efecto A ocurre antes o después del efecto B. En el caso de función grande, use Re Pág / Abajo para ubicar dos líneas y luego comparar Línea de números. En el caso de muchas funciones pequeñas, debe recordar en qué lugar del árbol de llamadas ocurren los efectos, y si olvidó, debe pasar una cantidad de tiempo no trivial redescubriendo la estructura de este árbol.

Al atravesar el árbol de llamadas de las funciones de soporte, también se enfrenta al desafío de determinar cuándo ha pasado de la lógica empresarial a los detalles de implementación. Afirmo sin evidencia * que cuanto más simple es el gráfico de llamadas, más fácil es hacer esta distinción.

(*) Al menos soy honesto al respecto ;-)

Una vez más, creo que ambos enfoques tienen fortalezas y debilidades.

Escriba solo funciones pequeñas si tiene que duplicar código

Estoy en desacuerdo. Como muestra su ejemplo de código, las funciones pequeñas y bien nombradas mejoran la legibilidad del código y deben usarse siempre que [por ejemplo] no le interese el "cómo", solo el "qué" de una funcionalidad.

Si está interesado en el "cómo" o el "qué" es una función del propósito para el que está leyendo el código (por ejemplo, obtener una idea general en lugar de rastrear un error). El propósito para el que está leyendo el código no está disponible mientras escribe el programa, y ​​lo más probable es que lea el código para diferentes propósitos; diferentes decisiones se optimizarán para diferentes propósitos.

Dicho esto, esta es la parte del punto de vista del jefe con la que probablemente no estoy de acuerdo.

No escriba una función con el nombre del comentario, coloque su línea de código compleja (3-4 líneas) con un comentario arriba. De esta manera, puede modificar el código que falla directamente

Realmente no puedo entender el razonamiento detrás de este, suponiendo que realmente sea serio. [...] Los comentarios tienen un defecto fundamental: no se compilan / interpretan y, por lo tanto, no se pueden probar de forma unitaria. El código se modifica y el comentario se queda solo y terminas sin saber cuál es el correcto.

Los compiladores solo comparan nombres para la igualdad, nunca te dan un Error de nombre engañoso. Además, debido a que varios sitios de llamadas pueden invocar una función determinada por su nombre, a veces es más arduo y propenso a errores cambiar un nombre. Los comentarios no tienen este problema. Sin embargo, esto es algo especulativo; para resolver esto realmente, uno probablemente necesitaría datos sobre si los programadores tienen más probabilidades de actualizar comentarios engañosos frente a nombres engañosos, y no tengo eso.

Jonas Kölker
fuente
-1

En mi opinión, el código correcto para la funcionalidad que necesita es:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER;

O si desea dividirlo en una función, probablemente algo como:

phoneNumber = getPhoneNumber(headers);

function getPhoneNumber(headers) {
  return headers.resourceId || DEV_PHONE_NUMBER
}

Pero creo que tiene un problema más fundamental con el concepto de "en producción". El problema con su función isApplicationInProductiones que parece extraño que este sea el único lugar en el sistema donde es importante estar en "producción", y que siempre puede confiar en la presencia o ausencia del encabezado resourceId para informarle. Debe haber un isApplicationInProductionmétodo general o un getEnvironmentmétodo que verifique el entorno directamente. El código debería verse así:

function isApplicationInProduction() {
  process.env.NODE_ENV === 'production';
}

Luego puede obtener el número de teléfono con:

phoneNumber = isApplicationInProduction() ? headers.resourceId : DEV_PHONE_NUMBER;
rjmunro
fuente
-2

Solo un comentario en dos de los puntos

  • Escribir pequeñas funciones es una molestia porque te obliga a pasar a cada pequeña función para ver qué está haciendo el código.
  • Ponga todo en un bucle principal principal, incluso si el bucle principal tiene más de 300 líneas, es más rápido de leer.

Muchos editores (por ejemplo, IntelliJ) le permitirán saltar a una función / clase simplemente presionando Ctrl y haciendo clic en el uso. Además, a menudo no necesita conocer los detalles de implementación de una función para leer el código, lo que agiliza la lectura del código.

Te recomiendo que le digas a tu jefe; le gustará su defensa y la verá como liderazgo. Solo sé cortés.

noɥʇʎԀʎzɐɹƆ
fuente