¿Puede / puede alguien desafiar al Tío Bob por su amor por eliminar "frenillos inútiles"?

93

No me gusta hacer referencia a contenido de pago, pero este video muestra exactamente de lo que estoy hablando. Precisamente 12 minutos en Robert Martin mira esto:

Ydice"Una de mis cosas favoritas para hacer es deshacerme de aparatos inútiles" a medida que lo convierte en esto:

Hacemuchotiempo,enunaeducaciónmuylejana,meenseñaronanohacerestoporquefacilitalaintroduccióndeunerroragregandootralíneaconsangría,pensandoqueestácontroladaporifcuandonoloestá.

ParaserjustoconeltíoBob,estárefactorizandounmétodoJavalargohastapequeñasfuncionesque,estoydeacuerdo,sonmuchomáslegibles.Cuandoterminadecambiarlo,(22.18)seveasí:

Mepreguntosiesosesuponequedebevalidarlaeliminacióndelasllaves.Yaestoyfamiliarizadoconlas mejores prácticas . ¿Puede el tío Bob ser desafiado en este punto? ¿El tío Bob ha defendido la idea?

    
pregunta candied_orange 04.06.2016 - 01:03
fuente

10 respuestas

46

La legibilidad no es poca cosa.

Soy de una mente mixta cuando se trata de llaves que encierran un solo método. Personalmente los elimino para cosas como declaraciones de devoluciones de una sola línea, pero al omitir esas llaves me mordió mucho en el último lugar donde trabajé. Alguien agregó una línea de código a una declaración if sin agregar también las llaves necesarias, y debido a que era C, falló el sistema sin previo aviso.

Nunca cuestiono a nadie que sea religioso sobre el uso de llaves después de ese pequeño fiasco.

Por lo tanto, veo el beneficio de la legibilidad, pero soy muy consciente de los problemas que pueden surgir cuando se omiten esas llaves.

No me molestaría en tratar de encontrar un estudio o la opinión publicada de alguien. Todo el mundo tiene uno (una opinión, eso es), y debido a que es un tema estilístico, una opinión es tan buena como cualquier otra. Piensa en el problema por ti mismo, evalúa los pros y los contras y toma tu propia decisión. Si la tienda para la que trabaja tiene un estándar de codificación que cubre este problema, simplemente siga eso.

    
respondido por el Robert Harvey 04.06.2016 - 02:07
fuente
132

Puede encontrar varias promociones o rechazos publicados de estilos sin llaves en aquí o aquí o dondequiera que estén pintados los cobertizos para bicicletas.

Alejándose de los cobertizos para bicicletas, ¿recuerdas el gran error de OS X / iOS SSL de 2014?

if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

Sí, "causado" por los bloques sin refuerzo enlace

Las preferencias pueden depender del estilo de corsé. Si tuviera que escribir

if (suiteSetup)
{
    includePage(mode, suiteSetup);
}

También me inclino a ahorrar espacio. Pero

if (suiteSetup) {
    includePage(mode, suiteSetup);
}

solo usa una línea "extra".

Sé que no preguntaste, pero si trabajo solo, soy un hereje. Si quito los frenos, prefiero

if (suiteSetup != null) includePage(mode, suiteSetup);

No tiene el mismo problema visual que el error del estilo SSL de iOS, pero guarda aún más líneas "innecesarias" que el tío Bob;) Lee bien si estás acostumbrado y tiene un uso general en Ruby (%código%). Oh, bueno, solo sepan que hay muchas opiniones.

    
respondido por el Paul Draper 04.06.2016 - 06:27
fuente
61

El tío Bob tiene muchas capas de defensa contra tal error que no era tan común cuando la sabiduría prevaleciente era "siempre usar llaves":

  1. Una fuerte preferencia personal por los condicionales de una sola línea, por lo que los de multilínea se destacan y reciben un escrutinio adicional.
  2. Un editor que supera automáticamente cuando inserta una segunda línea.
  3. Un conjunto completo de pruebas unitarias que ejecuta constantemente.
  4. Un conjunto completo de pruebas de integración.
  5. Una revisión por pares realizada antes de que su código se fusione.
  6. Una repetición de todas las pruebas por un servidor de integración continua.
  7. Pruebas realizadas por un departamento de calidad del producto.

Creo que si alguien publicara un desafío para el tío Bob, tendría una buena respuesta con los puntos anteriores. Este es uno de los errores más fáciles de detectar temprano.

    
respondido por el Karl Bielefeldt 04.06.2016 - 02:57
fuente
31

En su mayor parte, esta es una preferencia personal, sin embargo, hay algunas cosas que considerar.

Posibles errores

Aunque se puede argumentar que los errores causados por el olvido de las llaves de complemento son raros, por lo que he visto that czafuerza de los animales en la caja de seguridad. they do suceda IOS goto fail error). Así que creo que esto debería ser un factor al considerar el estilo de su código (algunas herramientas advierten sobre engaño-indentación , por lo que también depende de la cadena de herramientas) .

Código válido (que dice que podría ser un error)

Incluso suponiendo que su proyecto no sufra tales errores, al leer el código puede ver algunos bloques de código que parecen que podrían ser errores, pero no lo son, tomando parte de su estado mental ciclos.

Comenzamos con:

if (foo)
    bar();

Un desarrollador agrega un comentario útil.

if (foo)
    // At this point we know foo is valid.
    bar();

Más tarde un desarrollador se expande en él.

if (foo)
    // At this point we know foo is valid.
    // This never fails but is too slow even for debug, so keep disabled.
    // assert(is_valid(foo));
    bar();

O agrega un bloque anidado:

if (foo)
    while (i--) {
        bar(i);
        baz(i);
    }

O usa una macro:

if (foo)
    SOME_MACRO();

"... Ya que las macros pueden definir múltiples líneas de código, ¿la macro usa do {...} while (0) para múltiples líneas? Debería hacerlo porque está en nuestra guía de estilo, ¡pero es mejor que lo compruebe por si acaso!"

Los ejemplos anteriores son todos códigos válidos, sin embargo, cuanto más contenido haya en el bloque de código, más necesitará leer para asegurarse de que no haya errores.

Tal vez su estilo de código define que los bloques multilínea requieren un corchete (no importa qué, incluso si no son código) , pero he visto este tipo de comentarios agregados en Codigo de producción. Cuando lo lees, existe una pequeña duda de que quien haya editado esas líneas por última vez se olvidó de agregar un refuerzo, a veces siento que la necesidad de volver a verificar funciona como es debido (especialmente cuando se investiga un error en esta área del código ) .

Diff Noise

Una razón práctica para usar llaves para líneas simples es reducir diff noise .

Es decir, cambiando:

if (foo)
    bar();

Para:

if (foo) {
    bar();
    baz();
}

... hace que la línea condicional se muestre en una diferencia a medida que se cambia, esto agrega una pequeña sobrecarga innecesaria.

  • las líneas se muestran como cambiadas en las revisiones de código, si sus herramientas de diferenciación están basadas en palabras, puede ver fácilmente que solo se modificó la llave, pero eso toma más tiempo para verificar que la línea no cambió en absoluto . Habiendo dicho eso, no todas las herramientas son compatibles con diffing basado en palabras, diff (svn, git, hg ... etc) se mostrará como si la línea completa hubiera cambiado, incluso con herramientas sofisticadas, a veces es posible que tenga que buscar rápidamente sobre una línea simple basada en diferencias para ver qué ha cambiado.
  • las herramientas de anotación (como git blame ) mostrarán la línea como cambiada, haciendo que el seguimiento del origen de una línea sea más paso para encontrar el cambio real .

Ambos son pequeños y dependen de la cantidad de tiempo que invierta en revisar o rastrear el código de confirmación de las líneas de código cambiadas.

Un inconveniente más tangible de tener cambios de líneas adicionales en una diferencia, su mayor probabilidad de que los cambios en el código causarán conflictos que se fusionan y deben ser resueltos manualmente .

Hay una excepción a esto, para las bases de código que tienen { en su propia línea, no es un problema.

El argumento diff noise no se mantiene si escribe en este estilo:

if (foo)
{
    bar();
    baz();
}

Sin embargo, esta no es una convención tan común, por lo que se agrega principalmente a la respuesta para completarla (no sugiero que los proyectos deberían usar este estilo) .

    
respondido por el ideasman42 04.06.2016 - 08:11
fuente
15

Hace años, me trajeron para depurar algunos códigos C. El error fue una locura difícil de encontrar, pero finalmente se redujo a una declaración como:

if (debug)
   foo (4);

Y resultó que la persona que lo había escrito había definido foo como una macro. Una macro con dos líneas de código en ella. Y, por supuesto, solo la primera de esas dos líneas estuvo sujeta al if . (Así que la segunda línea fue ejecutada incondicionalmente).

Esto puede ser absolutamente único para C y su preprocesador, que realiza sustituciones antes de la compilación, pero nunca lo he olvidado. Ese tipo de cosas deja una marca en usted, y ¿por qué no jugar de forma segura, especialmente si usa una variedad de idiomas y cadenas de herramientas y no puede estar seguro de que esos chanchullos no sean posibles en otro lugar?

Al parecer, sangro y uso llaves de todos los demás. Para una sola línea if , haría:

if (debug) { foo (4); }

para que no tome ninguna línea adicional para incluir las llaves.

    
respondido por el Wayne 04.06.2016 - 15:49
fuente
14

"Tío Bob" tiene permitido tener su opinión, tú puedes tener tu opinión. No hay necesidad de desafiarlo.

Si desea apelar ante la autoridad, tome a Chris Lattner. En Swift, si las declaraciones pierden sus paréntesis, pero siempre vienen con llaves. Sin discusión, es parte del lenguaje. Así que si "Tío Bob" comienza a eliminar llaves, el código deja de compilar.

Pasar por el código de otra persona y "deshacerse de aparatos inútiles" es un mal hábito. Solo causa trabajo adicional cuando el código necesita ser revisado y cuando se crean conflictos innecesariamente. ¿Tal vez el "tío Bob" es un programador tan increíblemente bueno que no necesita revisiones de código? Perdí una semana de mi vida porque uno de los mejores programadores cambió "if (p! = NULL)" a "if (! P)" sin una revisión de código, oculto en el peor lugar posible.

Esto es principalmente un debate de estilo inofensivo. Las llaves tienen la ventaja de que puedes insertar otra línea de código sin agregar llaves. Al igual que una declaración de registro, o un comentario (si va seguido de un comentario seguido de una declaración es simplemente horrible). La declaración en la misma línea tiene la desventaja práctica de que tiene problemas con muchos depuradores. Pero haz lo que prefieras.

    
respondido por el gnasher729 05.06.2016 - 09:43
fuente
8

Mis razones para no eliminar llaves son:

  1. reduce la fatiga de decisión. Si siempre usa aparatos ortopédicos, nunca tiene que decidir si los va a necesitar o no.

  2. reduce la resistencia al desarrollo: incluso si te esfuerzas por eventualmente extraer todas las líneas de lógica a métodos, tener que convertir un braceless si a un brace si agregar lógica es un poco molesto desarrollo de arrastre. Por lo tanto, existe el esfuerzo de eliminar las llaves y el esfuerzo de agregarlas nuevamente cuando necesite más código. Diminuto, pero molesto.

respondido por el Michael Johnston 06.06.2016 - 08:08
fuente
0

Un joven compañero de trabajo ha dicho que las llaves que consideramos redundantes y superfluas son, de hecho, útiles para él. No recuerdo exactamente por qué, pero si le permite escribir mejor el código más rápido, solo eso es motivo para mantenerlos.

Fwiw, acordamos un compromiso de que ponerlos en una línea no hace que las condiciones previas tan breves un sean legibles / me distraigan. Por ejemplo,

if (!param) { throw invalid_argument (blahblah); }
if (n < 2) { return 0; }
// real code for function starts here...

También señalo que estas condiciones previas introductorias a menudo son declaraciones de flujo de control para el cuerpo (por ejemplo, un return ), por lo que el temor de agregar una segunda declaración que debe estar en la misma condición y olvidarse de escribir llaves es discutible . Una segunda declaración bajo la condición sería un código muerto de todos modos y no tendría sentido.

Sospecho que el problema de fluidez en la lectura se debe a que el analizador cerebral de la persona está conectado para tener las llaves como parte de la declaración condicional. Esa no es una representación precisa de la gramática de C ++, pero podría ser un efecto secundario de aprender primero otros idiomas, donde ese es el caso.

    
respondido por el JDługosz 05.06.2016 - 21:19
fuente
-1

Habiendo visto ese video justo ahora, me di cuenta de que eliminar los frenos hace que sea más fácil ver dónde aún se necesita refactuar el código. Si necesita llaves, su código probablemente sea un poco más limpio.

Habiendo dicho eso, cuando estás en un equipo, la eliminación de llaves probablemente conducirá a un comportamiento inesperado algún día. Digo que se los guarden, y te ahorrarás muchos dolores de cabeza cuando las cosas vayan mal.

    
respondido por el Gijs Overvliet 05.10.2016 - 11:47
fuente
-3
  

Me enseñaron a no hacer esto porque facilita la introducción de una   error al agregar otra línea con sangría pensando que está controlada por el si   cuando no lo es.

Si su código está bien probado en unidades , este tipo de "error" explotará en la cara.

Limpiar el código evitando cualquier soporte inútil y feo es una práctica que sigo.

    
respondido por el Mik378 06.06.2016 - 03:22
fuente

Lea otras preguntas en las etiquetas