Soy un gran creyente en la Regla de los Boy Scouts :
Siempre revise un módulo más limpio que cuando lo revisó ". No importa quién sea el autor original, ¿qué pasaría si siempre hiciéramos algún esfuerzo, no importa cuán pequeño, para mejorar el módulo? ¿Cuál sería el resultado? Creo que si todos seguían esa simple regla, veríamos el final del incesante deterioro de nuestros sistemas de software. En cambio, nuestros sistemas mejorarían gradualmente a medida que evolucionaran. También veríamos equipos cuidando el sistema en su conjunto, más bien que solo las personas que cuidan su propia pequeña parte.
También creo firmemente en la idea relacionada de la refactorización oportunista :
Aunque hay lugares para algunos esfuerzos de refactorización programados, prefiero alentar la refactorización como una actividad oportunista, realizada cuando sea y donde sea que el código necesite ser limpiado, por quien sea. Lo que esto significa es que en cualquier momento que alguien vea algún código que no sea tan claro como debería ser, debería aprovechar la oportunidad para solucionarlo allí mismo o en ese momento, o al menos en unos minutos.
Particularmente tenga en cuenta el siguiente extracto del artículo de refactorización:
Tengo cuidado con las prácticas de desarrollo que causan fricción para la refactorización oportunista ... Mi sensación es que la mayoría de los equipos no refactorizan lo suficiente, por lo que es importante prestar atención a cualquier cosa que desaliente a las personas a hacerlo. Para ayudar a eliminar esto, tenga en cuenta cada vez que se sienta desanimado de realizar una pequeña refactorización, una que está seguro que solo tomará uno o dos minutos. Cualquier barrera de este tipo es un olor que debería provocar una conversación. Por lo tanto, tome nota del desánimo y créelo con el equipo. Por lo menos, debe discutirse durante su próxima retrospectiva.
Donde trabajo, hay una práctica de desarrollo que causa mucha fricción: la revisión de código (CR). Cada vez que cambio algo que no está dentro del alcance de mi "asignación", mis revisores me reprochan que haga que el cambio sea más difícil de revisar. Esto es especialmente cierto cuando se trata de refactorizar, ya que dificulta la comparación de diferencias "línea por línea". Este enfoque es el estándar aquí, lo que significa que la refactorización oportunista rara vez se realiza, y solo se lleva a cabo la refactorización "planificada" (que generalmente es muy poco, demasiado tarde), si es que se realiza.
Afirmo que los beneficios valen la pena, y que 3 revisores trabajarán un poco más duro (para comprender realmente el código antes y después, en lugar de analizar el alcance limitado de qué líneas cambiaron; la revisión en sí misma sería mejor solo por eso ) para que se beneficien los próximos 100 desarrolladores que leen y mantienen el código. Cuando presento este argumento, mis revisores dicen que no tienen ningún problema con mi refactorización, siempre que no esté en el mismo CR. Sin embargo, afirmo que esto es un mito:
(1) La mayoría de las veces solo te das cuenta de qué y cómo quieres refactorizar cuando estás en medio de tu tarea. Como dice Martin Fowler:
A medida que agrega la funcionalidad, se da cuenta de que parte del código que está agregando contiene cierta duplicación con algún código existente, por lo que debe refactorizar el código existente para limpiar las cosas ... Puede que algo funcione, pero tenga en cuenta que sería mejor si se cambia la interacción con las clases existentes. Aproveche la oportunidad de hacer eso antes de considerarse terminado.
(2) Nadie te mirará favorablemente al liberar CR "refactorizadoras" que no debías hacer. Un CR tiene ciertos gastos generales y su gerente no quiere que "pierda su tiempo" en la refactorización. Cuando se incluye con el cambio que se supone que debes hacer, este problema se minimiza.
Resharper exacerba el problema, ya que cada archivo nuevo que agrego al cambio (y no puedo saber de antemano exactamente qué archivos terminarían modificados) generalmente está lleno de errores y sugerencias, la mayoría de los cuales son correctos y merecen totalmente fijación.
El resultado final es que veo un código horrible, y lo dejo allí. Irónicamente, siento que arreglar ese código no solo no mejorará mi posición, sino que en realidad los bajará y me pintará como el tipo "desenfocado" que pierde el tiempo arreglando cosas que a nadie le importan en lugar de hacer su trabajo. Me siento mal porque realmente desprecio el mal código y no puedo soportar verlo, ¡mucho menos llamarlo desde mis métodos!
¿Alguna idea sobre cómo puedo remediar esta situación?
fuente
your manager doesn't want you to "waste your time" on refactoring
Respuestas:
OK, ahora hay más cosas aquí de las que son adecuadas para un comentario.
tl; dr
Su intuición sobre lo que debe hacer (refactorizando a medida que avanza) es correcta.
Su dificultad para implementar esto, dado que debe evitar un sistema de revisión de código deficiente, se reduce a su dificultad para manipular su código fuente y VCS. Varias personas han dicho que puede y debe dividir sus cambios (sí, incluso dentro de los archivos) en múltiples confirmaciones, pero parece tener dificultades para creer esto.
Usted realmente puede hacer esto. Eso es realmente lo que estamos sugiriendo. Usted realmente debe aprender a sacar el máximo provecho de sus herramientas de edición, manipulación de la fuente y de control de versiones. Si invierte el tiempo en aprender a usarlos bien, la vida es mucho más fácil.
Flujo de trabajo / cuestiones de política de oficina
Recomendaría esencialmente la misma recomendación que GlenH7 de que cree dos confirmaciones: una con solo refactorizaciones y (demostrable u obviamente) sin cambios funcionales, y otra con los cambios funcionales en cuestión.
Sin embargo, puede ser útil, si encuentra muchos errores, elegir una sola categoría de error para corregir dentro de un solo CR. Luego tiene una confirmación con un comentario como "código de deduplicación", "corregir errores tipo X" o lo que sea. Debido a que esto hace un solo tipo de cambio, presumiblemente en múltiples lugares, debería ser trivial de revisar. Significa que no puede corregir cada error que encuentre, pero puede hacer que sea menos doloroso pasar de contrabando.
Problemas de VCS
Dividir los cambios realizados en su fuente de trabajo en múltiples confirmaciones no debería ser un desafío. No ha dicho lo que está usando, pero los posibles flujos de trabajo son:
si estás usando git, tienes excelentes herramientas para esto
git add -i
para la puesta en escena interactiva desde la línea de comandosgit gui
y seleccionar líneas y líneas individuales para organizar (esta es una de las pocas herramientas GUI relacionadas con VCS que en realidad prefiero a la línea de comandos, y la otra es un buen editor de combinación de 3 vías)rebase -i
si no está usando git, su VCS puede tener herramientas para este tipo de cosas, pero no puedo aconsejarle sin saber qué sistema usa
Mencionó que está usando TFS, que creo que es compatible con git desde TFS2013. Puede valer la pena experimentar con un clon git local del repositorio para trabajar. Si está deshabilitado o no funciona para usted, aún puede importar la fuente en un repositorio git local, trabajar allí y usarlo para exporta tus confirmaciones finales.
puede hacerlo manualmente en cualquier VCS si tiene acceso a herramientas básicas como
diff
ypatch
. Es más doloroso, pero ciertamente posible. El flujo de trabajo básico sería:diff
para hacer un archivo de parche (contexto o unificado) con todos los cambios desde la última confirmaciónpatch
para reproducir uno de los archivos de revisión, confirmar esos cambiosAhora tiene dos confirmaciones, con sus cambios particionados adecuadamente.
Tenga en cuenta que probablemente haya herramientas para facilitar esta administración de parches, simplemente no las uso porque git lo hace por mí.
fuente
Asumiré que las Solicitudes de cambio son grandes y formales en su empresa. Si no es así, solo realice los cambios (cuando sea posible) en muchas confirmaciones pequeñas (como se supone que debe hacerlo).
¿Continuar haciendo lo que estás haciendo?
Quiero decir, todos tus pensamientos y deducciones son completamente correctos. Deberías arreglar las cosas que ves. La gente no planifica refactorizar lo suficiente. Y ese beneficio para todo el grupo es más importante que molestar a algunos.
Lo que puede ayudar es ser menos combativo. Las revisiones de código no deberían ser combativas "¿Por qué hicieron eso?", Deberían ser colaborativas "¡Hola chicos, mientras estuve aquí arreglé todas estas cosas!". Trabajar (con su líder / gerente si es posible) para cambiar esa cultura es difícil, pero bastante vital para crear un equipo de desarrollo de alto funcionamiento.
También puede trabajar (con su líder / gerente si es posible) para avanzar la importancia de estas ideas con sus colegas. Hazlo preguntando "¿por qué no les importa la calidad?" en lugar de preguntar "¿por qué siempre haces estas cosas inútiles?".
fuente
Tengo mucha simpatía por su situación, pero pocas sugerencias concretas. Por lo menos, quizás te convenza de que, por muy mala que sea la situación, podría ser peor. SIEMPRE puede ser peor. :-)
Primero, creo que tienes (al menos) dos problemas con tu cultura, no solo uno. Un problema es el enfoque de refactorización, pero las revisiones de código parecen ser un problema distinto. Trataré de separar mis pensamientos.
Revisiones de código
Estaba en un grupo que ODIAba las revisiones de código. El grupo se formó fusionando dos grupos de partes separadas de la compañía. Vengo del grupo que había estado haciendo revisiones de código durante varios años, con resultados generalmente buenos. La mayoría de nosotros creía que las revisiones de código eran un buen uso de nuestro tiempo. Nos fusionamos en un grupo más grande y, por lo que podíamos ver, ese "otro" grupo nunca había oído hablar de revisiones de código. Ahora todos estábamos trabajando en "su" base de código.
Las cosas no estaban bien cuando nos fusionamos. Las nuevas características se retrasaron entre 6 y 12 meses, año tras año. La acumulación de errores fue enorme, creciente y agotadora. La propiedad del código era fuerte, especialmente entre los "gurús" más importantes. Las "ramas de características" a veces duraban años y abarcaban algunos lanzamientos; a veces NADIE, pero un solo desarrollador vio el código antes de que llegara a la rama principal. En realidad, la "rama de características" es engañosa, ya que sugiere que el código estaba en algún lugar del repositorio. Más a menudo fue solo en el sistema individual del desarrollador.
La gerencia acordó que necesitábamos "hacer algo" antes de que la calidad se volviera inaceptablemente baja. :-) Su respuesta fue Revisiones de código. Las revisiones de código se convirtieron en un elemento oficial de "Thou Shalt", para preceder a cada registro. La herramienta que utilizamos fue la Junta de Revisión.
¿Cómo funcionó en la cultura que describí? Mejor que nada, pero fue doloroso y tomó más de un año alcanzar una especie de nivel mínimo de cumplimiento. Algunas cosas que observamos:
Pensé que iba a escribir algunos párrafos sobre Revisiones de Código, pero resulta que estaba escribiendo sobre cultura. Supongo que todo se reduce a esto: las revisiones de código pueden ser buenas para un proyecto, pero un equipo solo obtiene los beneficios que merece.
Refactorización
¿Mi grupo despreciaba la refactorización aún más de lo que odiaba las revisiones de código? ¡Por supuesto! Por todas las razones obvias que ya se han mencionado. ¡Estás perdiendo el tiempo con una revisión de código asquerosa, y ni siquiera estás agregando una función o arreglando un error!
Pero el código todavía necesitaba desesperadamente una refactorización. ¿Cómo proceder?
fuente
¿Por qué no haces ambas cosas, pero con commits separados?
Tus compañeros tienen un punto. Una revisión de código debe evaluar el código que fue cambiado por otra persona. No debe tocar el código que está revisando para otra persona, ya que sesga su rol como revisor.
Pero si ve una serie de problemas evidentes, hay dos opciones que puede seguir.
Si la revisión del código estuvo bien, permita que la parte que revisó se confirme y luego refactorice el código bajo un segundo registro.
Si la revisión del código tuvo problemas que necesitaban corrección, solicite al desarrollador que refactorice según las recomendaciones de Resharper.
fuente
Personalmente odio la idea de las revisiones de código post commit. La revisión del código debería suceder, a medida que realiza los cambios en el código. Lo que, por supuesto, estoy hablando aquí es la programación de pares. Cuando se empareja, obtiene la revisión de forma gratuita y obtiene una mejor revisión del código. Ahora, estoy expresando mi opinión aquí, sé que otros comparten esto, probablemente hay estudios que prueban esto.
Si puede hacer que sus revisores de códigos se emparejen con usted, el elemento combativo de la revisión de códigos debería evaporarse. Cuando comienza a hacer un cambio que no se entiende, la pregunta puede plantearse en el punto de cambio, discutirse y explorar alternativas que pueden conducir a mejores refactorizaciones. Obtendrá una revisión de código de mayor calidad ya que el par podrá comprender el alcance más amplio del cambio y no centrarse tanto en los cambios línea por línea, que es lo que obtiene con la comparación de códigos lado a lado.
Por supuesto, esta no es una respuesta directa al problema de que las refactorizaciones estén fuera del alcance del cambio en el que se está trabajando, pero esperaría que sus pares entiendan mejor el razonamiento detrás de los cambios si estuvieran allí cuando hiciera el cambio.
Por otro lado, suponiendo que esté haciendo TDD o alguna forma de refactor rojo verde, una forma de asegurarse de obtener el compromiso de su compañero es utilizar la técnica de emparejamiento de ping pong. Simplemente se explica que el controlador se gira para cada paso del ciclo RGR, es decir, el par 1 escribe una prueba fallida, el par 2 lo repara, el par 1 refactores, el par 2 escribe una prueba fallida ... y así sucesivamente.
fuente
Probablemente este problema refleje un problema mucho mayor con la cultura de la organización. La gente parece más interesada en hacer "su trabajo" correctamente que en mejorar el producto, probablemente esta compañía tiene una cultura de "culpa" en lugar de una cultura de colaboración y la gente parece más interesada en cubrirse a sí misma que en tener una visión completa del producto / compañía .
En mi opinión, tienes toda la razón, las personas que revisan tu código están completamente equivocadas si se han quejado porque "tocas" cosas fuera de "tu asignación", intenta convencer a esas personas pero nunca estar en contra de tus principios, para mí esto es La cualidad más importante de un verdadero profesional.
Y si hacer lo correcto le da números malos de alguna manera estúpida de corporaciones para evaluar su trabajo, ¿cuál es el problema ?, ¿quién quiere "ganar" este juego de evaluación en una empresa loca ?, ¡trate de cambiarlo !, y si es imposible o estás cansado, encuentras otro lugar, pero nunca, nunca estés en contra de tus principios, es lo mejor que tienes.
fuente
A veces, refactorizar es algo malo. Sin embargo, no por las razones que sus revisores de código están dando; parece que realmente no les importa la calidad del código, y a usted sí le importa, lo cual es increíble. Pero hay dos razones que deberían impedirle refactorizar : 1. No puede probar los cambios que realizó con pruebas automáticas (pruebas unitarias o lo que sea) o 2. Está realizando cambios en algún código que no comprende muy bien. bien; es decir, no tienes el conocimiento específico del dominio para saber qué cambios debes hacer.
1. No refactorice cuando no pueda probar los cambios que realizó con las pruebas automatizadas.
La persona que realiza la revisión de su código debe poder asegurarse de que los cambios que realizó no rompan nada de lo que solía funcionar. Esta es la razón más importante para dejar solo el código de trabajo. Sí, definitivamente sería mejor refactorizar esa función de 1000 líneas de largo (¡eso es realmente una abominación!) En un montón de funciones, pero si no puede automatizar sus pruebas, puede ser realmente difícil convencer a otros de que hizo todo derecho. Definitivamente he cometido este error antes.
Antes de refactorizar, asegúrese de que haya pruebas unitarias. Si no hay pruebas unitarias, ¡escriba algunas! Luego refactorice y sus revisores de código no tendrán excusas (legítimas) para estar molestos.
No refactorice piezas de código que requieran conocimientos específicos del dominio que no tiene.
El lugar donde trabajo tiene muchos códigos de ingeniería química. La base del código utiliza los modismos comunes a los ingenieros químicos. Nunca haga cambios que sean idiomáticos en un campo. Podría parecer una gran idea para cambiar el nombre de una variable llamada
x
amolarConcentration
, pero ¿adivinen qué? En todos los textos de química, la concentración molar se representa con unx
. Cambiarle el nombre hace que sea más difícil saber lo que realmente está sucediendo en el código para las personas en ese campo.En lugar de renombrar variables idiomáticas, simplemente ponga comentarios explicando cuáles son. Si son no idiomática, por favor, cambiar el nombre de ellos. No deje el
i
,j
,k
,x
,y
, etc. flotando cuando mejores nombres funcionarán.Regla general para las abreviaturas: si, cuando las personas hablan, usan una abreviatura, está bien usar esa abreviatura en el código. Ejemplos de la base del código en el trabajo:
Tenemos los siguientes conceptos que siempre abreviamos cuando hablamos de ellos: "Área de preocupación" se convierte en "AOC", "Explosión de nubes de vapor" se convierte en VCE, cosas así. En nuestra base de código, alguien refactorizó todas las instancias llamadas aoc a AreaOfConcern, VCE a vaporCloudExplosion, nPlanes a confiningPlanesCount ... lo que hizo que el código fuera mucho más difícil de leer para las personas que tenían el conocimiento específico del dominio. También he sido culpable de hacer cosas como esta.
Es posible que esto no se aplique en absoluto a su situación, pero tengo mis pensamientos sobre el tema.
fuente
Esta pregunta ahora tiene dos problemas distintos: la facilidad de revisar los cambios en las revisiones de código y el tiempo dedicado a la refactorización.
Como han dicho otras respuestas, ¿puede separar los registros de refactorización de los registros de cambio de código (no necesariamente como revisiones separadas)? Dependiendo de la herramienta que esté utilizando para hacer la revisión del código, debería poder ver las diferencias solo entre revisiones específicas (el Crucible de Atlassian definitivamente hace esto).
Si la refactorización es sencilla y hace que el código sea más fácil de comprender (que es todo lo que debe hacer si solo se refactoriza), entonces la revisión del código no debería tardar mucho en completarse y debería ser una sobrecarga mínima que se recupera en espadas cuando alguien tiene que venir y mirar el código nuevamente en el futuro. Si sus jefes no pueden entender esto, entonces es posible que deba empujarlos suavemente hacia algunos recursos que discutan por qué la Regla Boy Scout conduce a una relación más productiva con su código. Si puede convencerlos de que la "pérdida [de] su tiempo" ahora ahorrará dos, cinco o diez veces más tarde cuando usted / alguien más regrese a trabajar más en ese código, entonces su problema debería desaparecer.
¿Has intentado llamar la atención de tus colegas sobre estos temas y discutir por qué vale la pena solucionarlos? ¿Y alguno de ellos puede repararse automáticamente (suponiendo que tenga suficiente cobertura de prueba para confirmar que no ha roto cosas con la refactorización automática)? A veces no es "valioso su tiempo" realizar una refactorización si realmente es algo quisquilloso. ¿Todo tu equipo usa ReSharper? Si lo hacen, ¿tiene una política compartida sobre qué advertencias / reglas se aplican? Si no lo hacen, tal vez debería estar demostrando dónde la herramienta lo está ayudando a identificar áreas del código que son posibles fuentes de dolor en el futuro.
fuente
Puedo recordar hace 25 años más o menos el código de "limpieza" en el que estaba trabajando para otros fines, principalmente formateando los bloques de comentarios y las alineaciones de tabulación / columna para que el código sea ordenado y, por lo tanto, más fácil de entender (sin cambios funcionales reales) . Me gusta el código ordenado y ordenado. Bueno, los desarrolladores senior estaban furiosos. Resulta que usaron algún tipo de comparación de archivos (diff) para ver qué había cambiado, en comparación con sus copias personales del archivo, y ahora estaba dando todo tipo de falsos positivos. Debo mencionar que nuestra biblioteca de códigos estaba en un mainframe y no tenía control de versión: básicamente sobrescribió lo que había allí, y eso fue todo.
La moraleja de la historia? No se. Supongo que a veces no puedes complacer a nadie excepto a ti mismo. No estaba perdiendo el tiempo (en mis ojos): el código limpiado era más fácil de leer y entender. Es solo que las primitivas herramientas de control de cambios utilizadas por otros ponen un trabajo adicional de una sola vez en ellas. Las herramientas eran demasiado primitivas para ignorar el espacio / tabulación y los comentarios redistribuidos.
fuente
Si puede dividir el cambio solicitado y el refactor no solicitado en (muchas) confirmaciones separadas, como lo señalaron @Useless, @Telastyn y otros, entonces eso es lo mejor que puede hacer. Seguirá trabajando en un único CR, sin la sobrecarga administrativa de crear uno "refactorizador". Simplemente mantenga sus compromisos pequeños y centrados.
En lugar de darle algunos consejos sobre cómo hacerlo, prefiero indicarle una explicación mucho más amplia (de hecho, un libro) de un especialista. De esta manera, podrás aprender mucho más. Lea Trabajo efectivo con código heredado (por Michael Feathers) . Este libro puede enseñarle cómo refactorizar, entrelazado con los cambios funcionales.
Los temas cubiertos incluyen:
Este libro también incluye un catálogo de veinticuatro técnicas de ruptura de dependencia que lo ayudan a trabajar con elementos del programa de manera aislada y hacer cambios más seguros.
fuente
Yo también soy un gran creyente en The Boyscout Rule y Opportunistic Refactoring. El problema a menudo está en lograr que la gerencia compre. La refactorización conlleva riesgos y costos. Lo que la administración a menudo pasa por alto es que también lo hace la deuda técnica.
Es la naturaleza humana. Estamos acostumbrados a enfrentar problemas reales, no a tratar de evitarlos. Somos reactivos, no proactivos.
El software es intangible y, por lo tanto, es difícil entender que, como un automóvil, necesita mantenimiento. Ninguna persona razonable compraría un automóvil y nunca lo repararía. Aceptamos que tal negligencia disminuiría su longevidad y, a la larga, sería más costosa.
A pesar del hecho de que muchos gerentes entienden esto, son responsables de los cambios en el código de producción. Hay políticas que hacen que sea más fácil dejar las cosas en paz. A menudo, la persona que necesita ser convincente está realmente por encima de su gerente y simplemente no quiere luchar para que sus "grandes ideas" entren en producción.
Para ser honesto, su gerente no siempre está convencido de que su remedio es, de hecho, tan bueno como cree que es. (¿Aceite de serpiente?) Hay ventas. Es su trabajo ayudarlo a ver el beneficio.
La gerencia no comparte sus prioridades. Póngase su sombrero de gestión y vea con ojos de gestión. Lo más probable es que encuentres el ángulo correcto. Se persistente. Efectuará más cambios al no permitir que la primera respuesta negativa lo detenga.
fuente
Si puede dividir el cambio solicitado y el refactor no solicitado en dos solicitudes de cambio separadas, como lo señaló @ GlenH7, entonces eso es lo mejor que puede hacer. Entonces no eres "el tipo que pierde el tiempo" sino "el tipo que trabaja el doble de duro".
Si a menudo se encuentra en una situación en la que no puede dividirlos, porque el trabajo solicitado ahora necesita el refactor no solicitado para compilar, le sugiero que insista en tener herramientas para verificar automáticamente los estándares de codificación, utilizando los argumentos descritos aquí (el Resharper las advertencias por sí solas pueden ser un buen candidato, no estoy familiarizado con eso). Todos esos argumentos son válidos, pero hay uno adicional que puede usar para su ventaja: tener esas pruebas ahora hace que sea su deber aprobar las pruebas en cada confirmación.
Si su empresa no quiere "perder el tiempo refactorizando" (una mala señal de su parte), entonces deben proporcionarle un archivo de "supresión de advertencia" (cada herramienta tiene su propio mecanismo) para que no se moleste más con tales advertencias Digo esto para proporcionarle opciones para diferentes escenarios, incluso en el peor de los casos. También es mejor tener acuerdos claramente establecidos entre usted y su empresa, también sobre la calidad del código esperado, en lugar de suposiciones implícitas en cada lado.
fuente