¿Cómo puedo sugerir tácticamente mejoras al código mal diseñado de otros durante la revisión?

1. Comprueba tu motivación. Si crees que el código debería cambiarse, deberías poder articular alguna razón por qué crees que debería cambiarse. Y esa razón debería ser más concreta que "Lo habría hecho de otra manera" o "Es feo". Si no puede señalar algún beneficio que proviene de su cambio propuesto, entonces no tiene mucho sentido gastar tiempo (a.k.a. dinero) en cambiarlo.

2. Cada línea de código en el proyecto es una línea que debe mantenerse. El código debe ser tan largo como sea necesario para hacer el trabajo y ser fácil de entender, y ya no. Si puedes acortar el código sin sacrificar la claridad, eso es bueno. Si puede hacerlo mientras aumenta la claridad, eso es mucho mejor.

3. El código es como concreto: es más difícil cambiarlo después de haber estado sentado por un tiempo. Si es posible, sugiera sus cambios con anticipación, de modo que el costo y el riesgo de los cambios se minimicen.

4. Cada cambio cuesta dinero. El código de reescritura que funciona y es poco probable que deba cambiarse podría desperdiciarse el esfuerzo. Concentre su atención en las secciones que están más sujetas a cambios o que son más importantes para el proyecto.

5. La forma sigue la función y, a veces, viceversa. Si el código está desordenado, existe una mayor probabilidad de que también contenga errores. Busque esos errores y critique la funcionalidad defectuosa en lugar del atractivo estético del código. Las mejoras sugeridas que hacen que el código funcione mejor y facilitan la verificación del funcionamiento del código.

6. Diferenciar entre diseño e implementación. Una clase importante con una interfaz de mierda puede propagarse a través de un proyecto como el cáncer. No solo disminuirá la calidad del resto del proyecto, sino que también aumentará la dificultad de reparar el daño. Por otro lado, una clase con una interfaz bien diseñada pero con una mala implementación no debería ser un gran problema. Siempre puede volver a implementar la clase para un mejor rendimiento o confiabilidad. O, si funciona correctamente y es lo suficientemente rápido, puede dejarlo solo y sentirse seguro sabiendo que su crucero está bien encapsulado.

Para resumir todos los puntos anteriores: Asegúrese de que los cambios propuestos agreguen valor.

Hay un punto dulce para agregar valor a través de la refactorización. Los cambios deben cumplir tres cosas:

• mejorar el código que es probable que cambie
• aumentar la claridad
• cuesta menos esfuerzo

Consideraciones:

1. Sabemos que el código limpio es menos costoso de escribir y mantener, y es más divertido trabajar en él. Tu trabajo es vender esa idea a la gente de tu empresa. Piense como un vendedor, no como un gruñón arrogante (es decir, no como yo).
2. No puedes ganar, solo puedes perder menos.
3. Enfocarse en agregar valor real, no solo belleza. Me gusta que mi código se vea bien, pero a veces tengo que aceptar más los asuntos económicos.
4. Una buena manera de encontrar el punto ideal es seguir el Principio de Boy Scout: cuando trabaje en un área de código, siempre déjelo en mejor forma de la que lo encontró.
5. Una pequeña mejora es mejor que ninguna mejora.
6. Hacer buen uso de las herramientas automatizadas. Por ejemplo, las herramientas que solo limpian un poco el formato pueden hacer de world de la diferencia.
7. Venda otras ideas que incidentalmente mejoren la claridad del código. Por ejemplo, las pruebas unitarias fomentan la descomposición de métodos grandes en métodos más pequeños.

Si el código funciona sin errores graves, y es inminente una fecha límite importante (como en los efectos P & amp o L), es demasiado tarde para sugerir mejoras que requieran cambios importantes. Incluso las mejoras al código pueden crear riesgos para la implementación del proyecto. El momento de las mejoras era anterior en el proyecto, cuando había más tiempo para invertir en la futura solidez del código base.

La revisión del código tiene tres propósitos:

1. Buscando errores

2. Comprobando para ver dónde se puede mejorar el código

3. Herramienta de enseñanza para quien haya escrito el código.

La evaluación de la calidad del diseño / código son, por supuesto, los números 2 y 3.

Hasta el # 2:

• Deje que quede MUY claro cuáles son los beneficios de los cambios propuestos frente a los costos que hay que corregir. Como cualquier decisión comercial, esto debe ser sobre el análisis de costo / beneficio.

  Por ejemplo, "El enfoque de X para el diseño reduciría significativamente la probabilidad de que el error Y se produzca al realizar el cambio Z, y sabemos que esta pieza de código sufre cambios de tipo Z cada 2 semanas. El costo de manejar la interrupción de la producción del error Y + encontrar el error + corregir y liberar el costo de oportunidad fijo + de no entregar el siguiente conjunto de funciones es $A , mientras que el costo de limpiar el código ahora y el costo de oportunidad (por ejemplo, el precio del envío con retraso o con menos funciones) es $B . Ahora , evalúe, o mejor dicho, pida a su jefe de equipo / gerente que evalúe $A vs $B y decida.

  • Esto ayudará a los líderes del equipo inteligente a gestionar esto de manera efectiva. P.ej. tomarán una decisión racional utilizando información COMPLETA

  • Esto (especialmente si redactas bien) aumentará TU estado, por ejemplo. usted es alguien lo suficientemente inteligente como para ver los beneficios de un mejor diseño, Y lo suficientemente inteligente como para no exigirlo religiosamente sin sopesar las consideraciones comerciales.

  • Y, en el caso probable de que ocurra el error Z, obtendrás mucho más apalancamiento en el siguiente conjunto de sugerencias.

Hasta el # 3:

• MUY claramente delinear los errores / problemas "debe arreglarse" de "Esta es una mejor práctica y realmente debería solucionarse SI podemos ahorrar recursos - vea las mejoras de diseño adjuntas en el análisis pro / con" (adjunte las cosas descritas para el # 2 arriba ) vs "Estas son pautas generales que creo que te ayudarán a mejorar la robustez de tu código para que puedas mantener el código con más facilidad" cambios opcionales. Tenga en cuenta la redacción: no se trata de "hacer que su código sea como lo que quiero"; es "si lo hace, USTED obtiene beneficios a, b, c". El tono y el enfoque son importantes.

Elige tus batallas, si se acerca un plazo, no hagas nada. La próxima vez que alguien más esté revisando o manteniendo el código y continúen teniendo problemas con él, entonces se les acerca la idea de que como equipo, debería estar más centrado en la calidad del código cuando revise el código para que no tenga tantos problemas más adelante.

Deben ver el valor antes de realizar el trabajo adicional.

Siempre comienzo mi comentario con "I would", lo que indica que el mío es simplemente uno de los muchos puntos de vista.

También siempre incluyo una razón.

" Yo extraería este bloque en un método porque de legibilidad".

Comento sobre todo; grande y pequeño. A veces hago más de cien comentarios sobre un cambio, en cuyo caso también recomiendo la programación en pares, y me ofrezco como ala-hombre.

Estoy tratando de establecer un lenguaje común por razones; legibilidad, SECO, SRP, etc.

También he creado una presentación en Código limpio y refactorización explicando por qué y mostrando cómo, lo que sostuve ante mis colegas. Lo he sostenido tres veces hasta ahora, y una consultora que estamos usando me pidió que lo guarde nuevamente para ellos.

Pero algunas personas no escucharán de todos modos. Entonces me quedo con tirar de rango. Soy el líder del diseño. La calidad del código es mi responsabilidad. Este cambio no se podrá ver en mi estado actual.

Tenga en cuenta que estoy más que dispuesto a retroceder en cualquier comentario que haga; Por razones técnicas, fechas límite, prototipos, etc., todavía tengo mucho que aprender sobre la codificación y siempre escucharé la razón.

Ah, y recientemente ofrecí comprar el almuerzo al primero de mi equipo que presentó un cambio no trivial en el que no tenía ningún comentario . (Oye, también tienes que divertirte. :-)

[Esta respuesta es un poco más amplia que la pregunta formulada originalmente, ya que esta es la redirección de muchas otras preguntas sobre revisiones de códigos.]

Aquí hay algunos principios que me parecen útiles:

Critique en privado, elogie públicamente. Permita que alguien sepa acerca de un error en su código uno a uno. Si hicieron algo brillante o asumieron una tarea que nadie deseaba, elogíalos en una reunión de grupo o en un correo electrónico enviado al equipo.

Comparta sus propios errores. Comparto la historia de mi desastrosa primera revisión de código (recibida) con estudiantes y colegas junior. También les digo a los estudiantes que atrapé su error tan rápido porque lo hice antes que yo. En una revisión del código, esto podría aparecer como: "Creo que las variables de índice están equivocadas aquí. Siempre lo verifico debido al tiempo en que obtuve los índices de forma incorrecta y derribé un centro de datos". [Sí, esta es una historia real.]

Recuerda hacer comentarios positivos. Un breve "¡bien!" o "buen truco!" Puede hacer el día de un programador junior o inseguro.

Supongamos que la otra persona es inteligente pero a veces descuidada. No digas: "¿Cómo esperas que la persona que llama obtenga el valor de retorno si no lo devuelves?" Diga: "Parece que se olvidó la declaración de devolución". Recuerda que escribiste código horrible en tus primeros días. Como alguien dijo una vez, "Si no te avergüenzas de tu código de hace un año, no estás aprendiendo".

Guarda el sarcasmo / ridículo para los amigos que no están en tu lugar de trabajo. Si el código es tremendamente horrible, bromea sobre eso en otro lugar. (Me parece conveniente estar casado con un compañero programador). Por ejemplo, no compartiría las siguientes caricaturas (o esta ) con mis colegas.

  

A veces me encuentro en una situación en la que el código de un compañero está plagado   Con diseño desordenado y muy poca preocupación por el mantenimiento futuro,   aunque es funcional y contiene poco o ningún error.

Este código está hecho. En cierto punto, los rediseños se vuelven demasiado costosos para justificarlos. Si el código ya es funcional con poco o ningún error, entonces esta será una venta imposible. Sugiera algunas formas de limpiar esto en el futuro y siga adelante. Si / cuando el código se rompe en el futuro, vuelva a evaluar el valor de un rediseño. Puede que nunca se rompa, lo que sería genial. De cualquier manera, estás en el punto en el que tiene sentido apostar que no se romperá, ya que el costo será el mismo ahora o más tarde: rediseñado, terrible rediseño.

Lo que debe hacer en el futuro es tener iteraciones de desarrollo más estrictas. Si hubiera podido revisar este código antes de que se hubiera invertido todo el trabajo de eliminar errores, habría tenido sentido sugerir un nuevo diseño. Hacia el final, nunca tiene sentido hacer una refactorización importante a menos que el código esté escrito de una manera fundamentalmente imposible de mantener y sabes con certeza que el código tendrá que cambiarse poco después del lanzamiento.

Dada la opción entre las dos opciones (refactor o no refactor), piensa en lo que parece ser la venta más inteligente:

  

Hola jefe, estábamos a tiempo y teníamos todo funcionando, pero ahora   vamos a reconstruir un montón de cosas para que podamos agregar la característica X en   el futuro.

o

  

Hola jefe, estamos listos para lanzar. Si alguna vez tenemos que agregar en función   X, nos puede llevar un par de días adicionales.

Si dijera alguno de los dos, su jefe probablemente diría:

  

¿Quién dijo algo sobre la característica X?

La conclusión es que a veces un poco de deuda técnica tiene sentido, si no fue capaz de corregir ciertas fallas cuando era barato (iteraciones tempranas). Tener un diseño de código de calidad tiene rendimientos decrecientes a medida que te acerques a una función completa y al plazo.

Cuando una cucharada de azúcar ayuda a que el medicamento baje, y lo que está mal se puede expresar de manera sucinta: no hay 20 cosas que estén mal. Me dirijo con una forma que sugiere que no tengo estaca ni ego en el que haya invertido. Lo que quiero ser escuchado. Por lo general, es algo como:

  

Me pregunto si sería mejor ...

o

  

¿Tiene algún sentido ...?

Si las razones son bastante obvias, no las declaro. Esto le da a otras personas la oportunidad de asumir cierta propiedad intelectual de la sugerencia, como en:

"Sí, es una buena idea, porque < su razón obvia aquí & gt ;."

Si la mejora es bastante obvia, pero no tanto como para hacerme ver como un idiota por no pensar en ello, y la razón para hacerlo refleja un valor compartido con el oyente, en algún momento ni siquiera lo sugiero. en su lugar:

Me pregunto si hay una manera de ... < declaración de valor compartido aquí >

Esto es solo para tratar con personas realmente sensibles, con la mayoría de mis compañeros, ¡simplemente les dejo que lo tengan!

Las revisiones de código no siempre tienen como objetivo realizar mejoras.

Una revisión cerca del final de un proyecto, como parece ser el caso aquí, es solo para que todos sepan dónde empezar a buscar cuando llegan los errores (o para un proyecto mejor diseñado que puede estar disponible para su posterior reutilización). Sea cual sea el resultado de la revisión, simplemente no hay tiempo para cambiar nada.

Para realizar cambios, debe estar discutiendo el código y el diseño mucho antes en el proyecto. El código es mucho más fácil de cambiar cuando solo existe para hablar sobre posibles enfoques.

Su pregunta es "¿Cómo codificar la revisión de un código mal diseñado?":

La respuesta IMO es simple. Hable sobre el DISEÑO del código y cómo el diseño es defectuoso o no cumple con los requisitos. Si señala un diseño defectuoso o "no cumple con los requisitos", entonces el desarrollador se verá obligado a cambiar su código porque no hace lo que tiene que hacer.

Si el código es "funcionalmente suficiente" y / o "cumple con las especificaciones" y / o "cumple con los requisitos":

Si eres un compañero de este desarrollador, no tienes ningún poder directo que te permita "decirle" que haga cambios.

Te quedan un par de opciones:

1. Debe usar su propia "influencia" personal (una forma de "poder" que es indirecta) y / o su capacidad de ser "persuasivo"
2. participe en el grupo de "procesos de código" de su organización y comience a hacer que el "mantenimiento de código" tenga una prioridad más alta.
3. Muerde la bala y aprenda a leer el código de mierda más rápido / con mayor fluidez para que no se cuelgue (parece que se le cuelga o disminuye la velocidad cuando se encuentra con el código de mierda) en el código de mierda.
   • Esto también te hará un programador más fuerte.
   • Y le permitirá corregir el código de mierda cuando esté trabajando en el código de mierda.
   • Y esto también te permitirá trabajar en más proyectos porque muchos proyectos tienen código de mierda que es funcional ... pero muchos códigos de mierda.
4. Liderar con el ejemplo. Mejora tu código ... pero no trates de ser un perfeccionista.
   • Porque entonces te convertirás en "el tipo lento que no puede cumplir con los plazos, siempre está criticando y piensa que es mejor que todos los demás".

Encuentro que no hay una bala de plata. Tienes que usar los tres y tienes que ser creativo en el uso de los tres.

En el caso de un diseño deficientemente paralizante, debe centrarse en maximizar la encapsulación . De esa forma, se vuelve más fácil reemplazar las clases / archivos / subrutinas individuales con clases mejor diseñadas.

Céntrese en garantizar que las interfaces públicas de los componentes estén bien diseñadas y que el funcionamiento interno esté cuidadosamente oculto. Además, los envoltorios de almacenamiento de datos son esenciales. (Grandes cantidades de datos almacenados pueden ser muy difíciles de cambiar, por lo que si tiene un "sangrado de implementación" en otras áreas del sistema, tiene problemas).

Una vez que haya superado las barreras entre los componentes, concéntrese en los componentes que probablemente causen problemas importantes.

Repita hasta la fecha límite o hasta que el sistema esté "perfecto".

En lugar de críticas directas directas al código de alguien, siempre es mejor ser constructivo en nuestros comentarios durante la revisión del código.

Una forma de seguir es

1. será óptimo si lo hacemos de esta manera.
2. Escribirlo de esta manera hará que se ejecute más rápido.
3. Tu código será mucho más legible si haces "esto" "esto" y "esto"

Dichos comentarios se tomarán en serio, aunque se cumplan los plazos. Y probablemente será implementado en el próximo ciclo de desarrollo.

La revisión del código debe integrarse con la cultura y el ciclo de desarrollo para funcionar. No es probable que la programación de una revisión de código grande al final del desarrollo de la característica X funcione. En primer lugar, hacer los cambios será más difícil y es probable que alguien se sienta avergonzado, creando resistencia a la actividad.

Debe tener confirmaciones tempranas y frecuentes, junto con revisiones en el nivel de confirmación. Con las herramientas de análisis de código en su lugar, la mayoría de las revisiones serán rápidas. Herramientas automatizadas de análisis / revisión de código como FindBugs y PMD te ayudará a eliminar una gran clase de errores de diseño. Sin embargo, no le ayudarán a resolver problemas de nivel arquitectónico, por lo que debe tener un diseño sólido en su lugar y juzgar el sistema general en función de ese diseño.

Aumente la calidad de las revisiones de código.

Aparte de la calidad del código que se está revisando, existe la calidad de la propia revisión del código:

• ¿Los cambios propuestos son realmente una mejora respecto a la presente?
• ¿O simplemente una cuestión de opinión?
• A menos que sea algo muy obvio, ¿el explicador lo ha explicado correctamente, ¿por qué ?
• ¿Cuánto tiempo tomó? (He visto revisiones que duraron meses, con el desarrollador bajo revisión a cargo de resolver todos los numerosos conflictos de combinación).
• ¿Tono?

Es mucho más fácil aceptar una revisión de código de buena calidad que una preparación del ego en su mayoría cuestionable.

Hay dos problemas notables en la pregunta, la parte con tacto y la parte que se aproxima . Estos son temas distintos: el primero es una cuestión de comunicación y dinámica de equipo, el segundo es una cuestión de planificación y priorización.

Con tacto . Supongo que desea evitar los egos cepillados y el rechazo negativo contra las revisiones. Algunas sugerencias:

• Tener un entendimiento compartido sobre los estándares de codificación y los principios de diseño.
• Nunca critique o revise el desarrollador , solo el código . Evite la palabra "usted" o "su código", solo hable sobre el código que se está revisando, separado de cualquier desarrollador.
• Ponga su orgullo en la calidad del código completado , de modo que se aprecien los comentarios que ayuden a mejorar el resultado final.
• Sugerir mejoras en lugar de demanda. Siempre ten un diálogo si no estás de acuerdo. Intente comprender el otro punto de vista cuando no esté de acuerdo.
• Haz que las revisiones vayan en ambos sentidos. Es decir. Haga que la persona que revisó revise su código a continuación. No tengo comentarios "de una sola vía".

La segunda parte es la priorización . Tiene muchas sugerencias para mejorar, pero como la fecha límite se acerca, solo hay tiempo para aplicar algunas.

Bueno, ¡primero quieres evitar que esto suceda en primer lugar! Esto se hace realizando revisiones continuas e incrementales. No permita que un desarrollador trabaje durante semanas en una función y luego revísela en el último momento. En segundo lugar, las revisiones de código y el tiempo para implementar las sugerencias de revisión deben formar parte de la planificación y estimaciones regulares para cualquier tarea. Si no hay suficiente tiempo para revisar adecuadamente, algo salió mal en la planificación.

Pero asumamos que algo salió mal en el proceso, y ahora se enfrenta a una serie de comentarios de revisión, y no tiene tiempo para implementarlos todos. Tienes que priorizar. Luego vaya a los cambios que serán más difíciles y más riesgosos de cambiar más adelante si los pospone.

El nombre de los identificadores en el código fuente es increíblemente importante para la legibilidad y la capacidad de mantenimiento, pero también es bastante fácil y de bajo riesgo cambiar en el futuro. Lo mismo con el formato de código. Así que no te concentres en eso. Por otro lado, la cordura de las interfaces expuestas públicamente debería ser la máxima prioridad, ya que son realmente difíciles de cambiar en el futuro. Los datos persistentes son difíciles de cambiar: si comienza a almacenar datos inconsistentes o incompletos en una base de datos, es muy difícil solucionarlos en el futuro.

Las áreas que están cubiertas por pruebas unitarias son de bajo riesgo. Siempre puedes arreglar eso luego. Las áreas que no lo son, pero que podrían ser probadas por unidad tienen un riesgo menor que las áreas que no pueden ser probadas por unidad.

Supongamos que tiene una gran cantidad de código sin pruebas unitarias y todo tipo de problemas de calidad de código, incluida una dependencia codificada de un servicio externo. En cambio, al inyectar esta dependencia, puede hacer que el fragmento de código sea verificable. Esto significa que en el futuro puede agregar pruebas y luego trabajar para solucionar el resto de los problemas. Con la dependencia codificada, ni siquiera se pueden agregar pruebas. Así que ve por esta solución primero.

¡En primer lugar, intenta evitar terminar en este escenario!