java - software - Refactorización de bucles internos con muchas dependencias entre niveles
refactorizar js (4)
No use regiones No ayudan si imprime, si usa CodeMap, etc.
Hacer list1, list2, .. private class variables.
Cree un método para cada bucle for()
:
private void method3()
{
for (item3 in list3)
{
list4 = service.get4(depending on item3 and list1);
method4();
}
}
Agregue algunos buenos comentarios para cada método ...
Esto tiene el beneficio adicional de permitir buenas pruebas unitarias de cada methodX()
Estoy teniendo el siguiente pre-código
using (some web service/disposable object)
{
list1 = service.get1();
list2 = service.get2();
for (item2 in list2)
{
list3 = service.get3(depending on item2);
for (item3 in list3)
{
list4 = service.get4(depending on item3 and list1);
for (item4 in list4)
{
...
}
}
}
}
donde tiene todo el código, digamos 500 líneas con mucha lógica adentro for
declaraciones. El problema es refactorizar esto en un código legible y mantenible y como una mejor práctica para situaciones similares. Aquí están las posibles soluciones que encontré hasta ahora.
1: dividir en métodos Considerando que cada uno se extrae en su propio método, para mejorar la legibilidad, terminamos con method2, method3 y method4. Cada método tiene sus propios parámetros y dependencias, lo cual es bueno, a excepción del método4. Method4 depende de list1, lo que significa que list1 se debe pasar a los métodos 2 y 3 también. Desde mi opinión, esto se vuelve ilegible. Cualquier desarrollador que mire method2 se dará cuenta de que no tiene sentido enumerar 1 en su interior, por lo que tiene que mirar hacia abajo hasta el método 4 para darse cuenta de la dependencia -> ineficaz. Entonces, ¿qué sucede si la lista4 o el elemento4 cambia y ya no necesita una dependencia en la lista1? Tengo que eliminar el parámetro list1 para method4 (que debe hacerse, naturalmente) pero también para los métodos 2 y 3 (fuera del alcance del cambio) -> nuevamente ineficaz. Otro efecto secundario es que, en el caso de muchas dependencias y niveles múltiples, la cantidad de parámetros transmitidos aumentará rápidamente. Piense qué pasa si list4 también depende de list11, list12 y list13, todos creados en el nivel de list1.
2: mantener un único método largo La ventaja es que cada lista y elemento puede acceder a cada lista y elemento principal, lo que hace que los cambios adicionales sean unidireccionales. Si list4 ya no depende de list1, simplemente elimine / reemplace el código, sin cambiar nada más. Obviamente, el problema es que el método es un par de cientos de líneas, que todos sabemos que no es bueno.
3. Lo mejor de ambos mundos? La idea es dividir en métodos solo la parte lógica interna de cada uno. De esta forma, el método principal disminuirá y obtenemos legibilidad y posiblemente capacidad de mantenimiento. Dejando el for
en el método principal, todavía podemos acceder a cada dependencia como una línea. Terminamos con algo como esto:
for (item2 in list2)
{
compute();
list3 = service.get3(depending on item2);
for (item3 in list3)
{
compute(item2, eventually item1)
list4 = service.get4(depending on item3 and list1);
for (item4 in list4)
{
...
}
}
}
Pero hay otro problema. ¿Cómo compute
esos métodos de compute
para ser legible? Digamos que tengo una variable local instanceA
o type ClassA
que puebla del item2. La clase A contiene una propiedad llamada LastItem4 que necesita conservar, digamos, el último item4 en orden de fecha de creación o lo que sea. Si utilizo compute para crear la instanceA
, entonces este cálculo es solo parcial. Porque la propiedad LastItem4 debe completarse en el nivel item4, en un método de cálculo diferente. Entonces, ¿cómo puedo llamar a estos 2 métodos de cálculo para sugerir lo que estoy haciendo en realidad? Respuestas difíciles, en mi opinión.
4. #region Deje un solo método largo pero use regiones. Esta es una característica estrictamente utilizada en algunos lenguajes de programación y me parece un parche, no como una mejor práctica, pero tal vez sea solo yo.
Me gustaría saber cómo harías esta refactorización? Tenga en cuenta que puede ser incluso más complejo que eso. El hecho de que es un servicio es un poco engañoso, la idea es que realmente no me gusta usar objetos desechables como parámetros de método, porque no se conoce el intento sin leer el código de método real. Pero esperaba esto en los comentarios, ya que otros pueden sentir lo contrario.
Por el bien del ejemplo, supongamos que el servicio es de terceros, sin la posibilidad de cambiar la forma en que funciona.
Parece que estás buscando una respuesta general en lugar de una respuesta específica. La clave de la respuesta general es separar las responsabilidades y esforzarse por lograr el principio de responsabilidad única .
Permítanme comenzar con el punto de que su método hace demasiadas cosas . Esto es algo que idealmente una clase tampoco debería hacer. Debería hacerse con la ayuda de un grupo de clases.
Robert C. Martin solía decir "Las clases tienden a ocultarse en los métodos", que es el caso aquí. Si su método es grande, puede refactorizarlo a métodos más pequeños, pero si se necesita una variable local en todo el método, probablemente ese método debe pertenecer a su propia clase.
Si necesita que se accedan algunos campos juntos, tienen que vivir juntos (me refiero a un mismo tipo). Entonces, toda tu Lista1, Lista2, Lista3, Lista4 debería vivir en una clase, no como parámetros del método en absoluto.
Dado que no puede modificar la clase de Service
(suponiendo que sea la API de terceros). Está bien, siempre puedes crear tu propia clase que envuelva el servicio antes mencionado.
Entonces, esencialmente, una clase como la siguiente:
public class ServiceWrapper
{
private IList<string> list1;
private IList<string> list2;
private IList<string> list3;
private IList<string> list4;
private readonly Service1 service;
public ServiceWrapper(Service1 service)
{
this.service = service;
}
public SomeClass GetSomething()
{
list1 = service.Get1();
list2 = service.Get2();
foreach (var item2 in list2)
{
PopulateMore(item2);
}
return result;//Return some computed result
}
private void PopulateMore(string item2)
{
list3 = service.Get3(item2);
foreach (var item3 in list3)
{
PopulateEvenMore(item3);
}
}
private void PopulateEvenMore(string item3)
{
list4 = service.Get4(item3);
foreach (var item4 in list4)
{
//And so on
}
}
}
Dado su servicio se ve algo como esto
public class Service1 : IDisposable
{
public void Dispose()
{
throw new NotImplementedException();
}
public IList<string> Get1()
{
throw new NotImplementedException();
}
public IList<string> Get2()
{
throw new NotImplementedException();
}
public IList<string> Get3(string item2)
{
throw new NotImplementedException();
}
public IList<string> Get4(string item3)
{
throw new NotImplementedException();
}
}
Entonces su método muy largo se vuelve muy pequeño ahora.
public void YourVeryLongMethodBecomesVerySmall()
{
using (Service1 service = new Service1())
{
SomeClass result = new ServiceWrapper(service).GetSomething();
}
}
Como dijo en los comentarios, si tiene otras responsabilidades también dentro de esos métodos, debe separar esas responsabilidades y moverlas en su propia clase. Por ejemplo, si desea analizar algo, debería ir dentro de alguna clase de Parser
y ServiceWrapper
debería usar el analizador para realizar el trabajo (idealmente con algo de abstracción y acoplamiento flexible).
Deberías extraer métodos y clases tanto como puedas para que no tenga sentido extraer más.
Terminé usando la solución 3 descrita en la pregunta, en combinación con los DTO. Esta no es necesariamente la mejor opción, pero es mejor que el resto en términos de legibilidad, flexibilidad y escalado.
Respuesta corta, depende del dominio de tu problema y de cuanto puedes dividir tu método en verbos que tengan sentido, pero generalmente la opción 3 es la mejor opción según mi experiencia.
Acerca de #region, no lo use. Se usa mejor para separar el código generado de tu código. Usar la región para "estructurar" tu código es como poner basura debajo de la alfombra, todavía está allí y puedes sentirlo.
Una cosa más a tener en cuenta es que probablemente se enfrenta a un problema relacionado con las colecciones y una buena biblioteca como Guava (google-collections) podría ayudarlo mucho. También el uso de las características Java 8 lambda o .NET LINQ podría ayudarlo a reescribir su código y hacerlo más expresivo.
- Guava: https://code.google.com/p/guava-libraries/
- Java 8 Streams vs .NET LINQ: http://blog.informatech.cr/2013/03/24/java-streams-preview-vs-net-linq/