"Cosmetic" clean-up of old, unknown code. ¿Qué pasos, qué orden? ¿Qué tan invasivo?

Question

Cuando recibo un código que no había visto antes para refactorizarlo, normalmente arreglo cosas "cosméticas" (como convertir StringTokenizers en String#split(), reemplazando las colecciones anteriores a la 1.2 por colecciones más nuevas, creando los campos final, convirtiendo el estilo C arrays a arrays tipo Java, ...) mientras leo el código fuente con el que me tengo que familiarizar.

¿Hay muchas personas que usan esta estrategia (tal vez es algún tipo de "mejor práctica" que no sé) o esto se considera demasiado peligroso, y no tocar el código antiguo si no es absolutamente necesario generalmente se prefiere ? ¿O es más común combinar el paso de "limpieza cosmética" con el paso más invasivo de "refactorización general"?

¿Cuáles son las "frutas más comunes" cuando se hace "limpieza cosmética" (frente a la refactorización con cambios más invasivos)?

Answer 1

En mi opinión, "limpieza cosmética" es "refactorización general." Solo está cambiando el código para que sea más comprensible sin cambiar su comportamiento.

Siempre refactorizo ​​atacando primero los cambios menores. Cuanto más legible pueda hacer el código rápidamente, más fácil será hacer los cambios estructurales más adelante, especialmente porque lo ayuda a buscar códigos repetidos, etc.

Normalmente, empiezo mirando el código que se usa con frecuencia y tendrá que ser cambiado a menudo, primero. (Esto tiene el mayor impacto en el menor tiempo ...) La denominación variable es probablemente la "fruta más fácil" para atacar primero, seguida de las actualizaciones del marco (cambios en la recopilación, métodos actualizados, etc.). Una vez hecho esto, el siguiente paso es dividir los métodos grandes, seguido de otras refactorizaciones típicas.

Answer 2

Lo primero que hago es tratar de ocultar la mayoría de las cosas al mundo exterior. Si el código es malo la mayoría del tiempo, el tipo que lo implementó no sabía mucho sobre el ocultamiento de datos y similares.

Así que mi consejo, lo primero que debe hacer:

Encienda el mayor número de miembros y métodos como privada como sea posible sin romper la compilación .

Como segundo paso trato de identificar las interfaces. Sustituyo las clases concretas a través de las interfaces en todos los métodos de clases relacionadas. De esta forma desacoplarás las clases un poco.

Refactorización adicional se puede hacer de forma más segura y local.

Answer 3

Puede comprar una copia de Refactoring: Improving the Design of Existing Code de Martin Fowler, encontrará muchas cosas que puede hacer durante su operación de refactorización.

Además, puede utilizar herramientas proporcionadas por su IDE y otros analizadores de código como Findbugs o PMD para detectar problemas en su código.

---

Recursos:

• www.refactoring.com
• wikipedia - List of tools for static code analysis in java

Sobre el mismo tema:

• How do you refactor a large messy codebase?
• Code analyzers: PMD & FindBugs

Answer 4

normalmente no me molesta ir a través de código de edad en busca de problemas. Sin embargo, si lo estoy leyendo, como pareces estar haciendo, y hace que mi cerebro falle, lo soluciono.

Las frutas comunes para mí tienden a ser más sobre cambiar el nombre de clases, métodos, campos, etc., y escribir ejemplos de comportamiento (también conocido como pruebas unitarias) cuando no puedo estar seguro de lo que una clase está haciendo por inspección En general, hacer que el código sea más legible a medida que lo leo. Ninguno de estos es lo que yo llamaría "invasivo", pero son más que simples cosméticos.

Answer 5

Estás en el camino correcto. Al hacer las correcciones pequeñas, estará más familiarizado con el código y las correcciones más grandes serán más fáciles de hacer con todos los detritus fuera del camino.

Ejecute una herramienta como JDepend, CheckStyle o PMD en la fuente. Pueden realizar automáticamente muchos cambios de tipo cosmético pero basados ​​en reglas generales de refactorización.

Answer 6

No hay una respuesta correcta o incorrecta aquí, ya que esto depende en gran medida de las circunstancias.

Si el código es en vivo, funciona, no está documentado y no contiene ninguna infraestructura de prueba, entonces no lo tocaría. Si alguien vuelve en el futuro y quiere nuevas funciones, intentaré integrarlas en el código existente cambiando al mínimo posible.

Si el código tiene errores, características problemáticas, faltantes, y fue escrito por un programador que ya no trabaja con la empresa, entonces probablemente rediseñaría y reescribiría todo. Siempre podría hacer referencia al código de ese programador para una solución específica a un problema específico, pero me ayudaría a reorganizar todo en mi mente y en la fuente. En esta situación, todo el asunto probablemente esté mal diseñado y podría utilizar una reconsideración completa.

Para todo lo demás, tomaría el enfoque que delineaste. Comenzaría limpiando todo cosméticamente para poder ver lo que está pasando. Entonces empezaría a trabajar en cualquier código que destacara como que necesitaba más trabajo. Agregaría documentación ya que entiendo cómo funciona para poder ayudar a recordar lo que está sucediendo.

En última instancia, recuerde que si va a mantener el código ahora, debería cumplir sus estándares. Donde no lo esté, debe tomarse el tiempo para llevarlo a sus estándares, lo que sea que eso tome. Esto le ahorrará mucho tiempo, esfuerzo y frustración en el futuro.

Answer 7

Al comenzar con la "limpieza cosmética" se obtiene una buena visión general de cuán desordenado es el código y esto combinado con una mejor legibilidad es un buen comienzo.

Siempre (sí, claro ... a veces hay algo que se llama una fecha límite que me ensucia) empiece con este enfoque y me ha servido muy bien hasta ahora.

Answer 8

No cambio el código anterior, excepto para formatearlo usando el IDE. Hay demasiado riesgo de introducir un error - o eliminando un error que otro código ahora depende de! O introduciendo una dependencia que no existía, como usar el montón en lugar de la pila.

Más allá del formato IDE, no cambio el código que el jefe no me ha pedido que cambie. Si algo es atroz, le pregunto al jefe si puedo hacer cambios y explicar un caso de por qué esto es bueno para la empresa.

Si el jefe me pide que corrija un error en el código, realizo los menos cambios posibles. Supongamos que el error está en un bucle simple. Refactorizaría el ciclo en un nuevo método. Luego, escribiría un caso de prueba para ese método para demostrar que encontré el error. Entonces arreglaría el nuevo método. Entonces me aseguraría de que los casos de prueba pasen.

Sí, soy un contratista. Contratación te da un punto de vista diferente. Lo recomiendo.

Answer 9

Por experiencia, depende de dos cosas: tiempo y riesgo.

Si tiene suficiente tiempo, puede hacer mucho más, de lo contrario, el alcance de los cambios que realice se reducirá en consecuencia. Por mucho que odie hacerlo, he tenido que crear algunos hacks vergonzosos horribles porque simplemente no tuve suficiente tiempo para hacerlo bien ...

Si el código en el que está trabajando tiene muchas dependencias o es crítico a la aplicación a continuación, hacer los menos cambios posibles - nunca se sabe lo que su solución podría romper ... :)

Parece que usted tiene una idea sólida de lo que las cosas deben parecerse a lo que no estoy va a decir qué cambios específicos hacer en qué orden porque eso variará de persona a persona. Simplemente haga pequeños cambios localizados primero, pruebe, expanda el alcance de sus cambios, pruebe. Expandir. Prueba. Expandir. Prueba. ¡Hasta que se te acabe el tiempo o no haya más lugar para mejorar!

BTW Durante las pruebas es probable que veas dónde las cosas se rompen con mayor frecuencia: crea casos de prueba para ellas (JUnit o lo que sea).

EXCEPCIÓN: Dos cosas que siempre me encuentro haciendo son reformatear (CTRL + SHFT + F en Eclipse) y comentar el código que no es obvio. Después de eso, simplemente martillo el clavo más obvio primero ...

Answer 10

La fruta cosmética más colgante es (en Eclipse, de todos modos) shift-control-F. El formateo automático es tu amigo.

Answer 11

Hay una cosa que debes tener en cuenta. El código que está comenzando ha sido PROBADO y aprobado, y sus cambios significan automáticamente que esa nueva prueba debe ocurrir ya que puede haber roto inadvertidamente algún comportamiento en otro lugar.

Además, todo el mundo comete errores. Cada cambio no trivial haces (cambiando StringTokenizer para dividir es no una función automática, por ejemplo en Eclipse, por lo que escribe usted mismo) es una oportunidad incurrir en errores en. Cómo se obtiene el derecho comportamiento exacto de un condicional, o ¿Por simple error olvidó un !?

Por lo tanto, los cambios implica un nuevo análisis. Ese trabajo puede ser bastante sustancial y severamente abrumar a los pequeños cambios que has hecho.