2010-05-26 17 views
33

tengo una clase Java con un método mil línea de if/else lógica de la siguiente manera:de perfeccionar/else lógica

if (userType == "admin") { 
    if (age > 12) { 
      if (location == "USA") { 
       // do stuff 
      } else if (location == "Mexico") { 
       // do something slightly different than the US case 
      } 
    } else if (age < 12 && age > 4) { 
      if (location == "USA") { 
       // do something slightly different than the age > 12 US case 
      } else if (location == "Mexico") { 
       // do something slightly different 
      } 
    } 
} else if (userType == "student") { 
    if (age > 12) { 
      if (location == "USA") { 
       // do stuff 
      } else if (location == "Mexico") { 
       // do something slightly different than the US case 
      } 
    } else if (age < 12 && age > 4) { 
      if (location == "USA") { 
       // do something slightly different than the age > 12 US case 
      } else if (location == "Mexico") { 
       // do something slightly different 
      } 
    } 

¿Cómo debería refactorizar esto en algo más manejable?

+18

Entiendo que este es un ejemplo rápido, pero la comparación de cadenas realmente debe hacerse con 'igual()'. – BalusC

+14

... ¿Qué sucede cuando tienes exactamente 12 años? – polygenelubricants

+2

Echaré un vistazo al patrón del decorador http://en.wikipedia.org/wiki/Decorator_pattern. – Lumpy

Respuesta

1

Puede hacer userType un enum, y darle un método que realice todas sus acciones "hacer algo ligeramente diferente".

24

Debe utilizar Strategies, posiblemente implementado dentro de una enumeración, por ejemplo:

enum UserType { 
    ADMIN() { 
    public void doStuff() { 
     // do stuff the Admin way 
    } 
    }, 
    STUDENT { 
    public void doStuff() { 
     // do stuff the Student way 
    } 
    }; 

    public abstract void doStuff(); 
} 

A medida que la estructura de código dentro de cada if rama externa en su código sea más o menos la misma, en el siguiente paso de refactorización que podría querer restar importancia a esa duplicación usando template methods. Alternativamente, puede convertir la ubicación (y posiblemente la edad) en una estrategia también.

Actualización: en Java4, puede implementar un typesafe enum a mano, y usar subclases viejas simples para implementar las diferentes estrategias.

1

sin más información no existe una buena respuesta

pero justo conjetura sería la siguiente: el uso OO

definir primero un usuario, definir de administración, estudiantes y todos los otros tipos de usuarios y luego dejar que el polimorfismo cuidar del resto

1

Basado sólo en los nombres de las variables, supongo que debe subclase User (o lo que sea que tiene una variable userType) en AdminUser y StudentUser (y posiblemente otros) y el uso polymorphism.

5

- Use enumeraciones para userType y la ubicación - a continuación, se pueden utilizar las instrucciones switch (mejora la legibilidad)

En segundo lugar - utilizan más métodos.

Ejemplo:

switch (userType) { 
    case Admin: handleAdmin(); break; 
    case Student: handleStudent(); break; 
} 

y más tarde

private void handleAdmin() { 
    switch (location) { 
    case USA: handleAdminInUSA(); break; 
    case Mexico: handleAdminInMexico(); break; 
    } 
} 

Además, identificar código duplicado y ponerlo en métodos adicionales.

EDITAR

Si alguien te obliga a código Java sin enumeraciones (como se ve obligado a utilizar Java 1.4.2), utiliza 'estática del último lugar de enumeraciones o hacer algo como:

if (isAdmin(userType)) { 
    handleAdmin(location, age); 
    } else if (isStudent(userType)) { 
    handleStudent(location, age)); 
    } 

//... 

private void handleAdmin(String location, int age) { 
    if (isUSA(location)) { 
    handleAdminInUSA(age); 
    } else if (isUSA(location)) { 
    handleAdminInMexico(age); 
    } 
} 

//... 

private void handleAdminInUSA(int age) { 
    if (isOldEnough(age)) { 
    handleAdminInUSAOldEnough(); 
    } else if (isChild(age)) { 
    handleChildishAdminInUSA(); // ;-) 
    } //... 
} 
+0

Se me olvidó mencionar que esta es una aplicación java 4 -> sin enumeraciones – David

+0

También crearía enum para ageGroup: INFANTIL, NIÑO, OVERTWELVE. – DJClayworth

+0

@David - bueno hazlo a la antigua usanza; p.ej. usando un HashMap para mapear las cadenas de tipo de usuario a constantes enteras, y luego activar las constantes enteras. –

1

El riesgo de esto no es solo que sea antiestético, sino que es muy propenso a errores. Después de un tiempo, podría correr el riesgo de superposiciones en sus condiciones.

Si realmente puede distinguir la condición por tipo de usuario, puede como mínimo dividir el cuerpo de cada condición en una función separada.Para que compruebe según el tipo, y llame a una función apropiada específica para ese tipo. Una solución más OO es representar a cada usuario como una clase, y luego anular algún método de cálculo para devolver un valor en función de la edad. Si no puede usar clases pero puede al menos usar enumeraciones, entonces podrá hacer una declaración de conmutación más agradable en las enumeraciones. Los interruptores en Cuerdas sólo vienen en Java 7.

Lo que me preocupa es situaciones de superposiciones (por ejemplo, dos tipos de usuarios con algunas reglas compartidas, etc.). Si ese fuera el caso, sería mejor que representara los datos como un archivo externo (por ejemplo, una tabla) que leería y mantendría, y su código operará esencialmente como un controlador que realiza la búsqueda adecuada en esta información. conjunto. Este es un enfoque común para las reglas comerciales complejas, ya que nadie quiere ir y mantener toneladas de código.

12

Lo primero que haría con este código es crear los tipos Admin y Student, que heredan del tipo base User. Estas clases deben tener un método doStuff() donde ocultas el resto de esta lógica.

Como regla general, cada vez que se coge que el cambio en el tipo, puede utilizar el polimorfismo en su lugar.

1

que probablemente primero comprobar si puede parametrizar el código hacerTarea y doSimilarStuff.

9

¿Miles? Tal vez un motor de reglas es lo que necesitas. Drools podría ser una alternativa viable.

o un patrón de comando que encapsula todo el "hacer algo diferente ligeramente" lógica para cada caso. Almacene cada comando en un mapa con la concatenación de edad, ubicación y otros factores como la clave. Busca el comando, ejecútalo y listo. Bonito y limpio.

El mapa se puede almacenar como configuración y leer en el inicio. Puede agregar nueva lógica agregando nuevas clases y reconfigurando.

+0

+1 para buscar en un motor de reglas. Si tiene miles de reglas, acelerarán en gran medida la ejecución y la administración. –

0

Conceptos Uso de POO: Esto depende del resto del diseño, pero tal vez debería tener una interfaz user, Student, Admin Interfaces la extiende y UsaStudent, MexicoStudent, UsaAdmin, MexicoAdmin implementación que hacer algunas cosas. Mantenga una instancia User y simplemente llame a su método doStuff.

+0

De esta manera, en lugar de mil líneas de "si-entonces-más", se encontrará con miles de clases con complejas relaciones de herencia. –

+0

@Artem Barger Prefiero muchas clases pequeñas en unos pocos métodos muy largos. –

+0

definiendo un montón de clases pequeñas simplemente eliminando el código de espagueti de un lugar a otro, después de todo, una vez que leas una entrada necesitarás decidir qué instancia crear, por lo que lo harás dentro de tu clase de fábrica o algo así. –

1

Eche un vistazo al patrón Visitor. Hace uso del polimorfismo pero es un poco más flexible ya que es más fácil agregar nuevos casos más adelante.

La desventaja es que había necesidad alguna manera de convertir la información del estado en diferentes instancias. El beneficio es una manera más limpia de agregar comportamiento sin tener que modificar su jerarquía de herencia.

1

Puede usar el patrón Cadena de responsabilidad.

Refactor if-else declaraciones en clases con una interfaz IUserController por ejemplo.

inicializar su cadena dentro de una lista o un árbol o cualquier estructura de datos adecuada, y ejecutar la funcionalidad deseada en esta cadena. Puede usar el patrón de Constructor para crear la estructura de datos mencionada. Se asemeja al patrón de estrategia, pero en el patrón de cadena de responsabilidad, una instancia en la cadena puede llamar instancia (s) vinculada (s).

Además, puede modelar la funcionalidad específica de la ubicación mediante el patrón de estrategia. Espero eso ayude.

1

Si el código en los bloques se ajusta a unos patrones estándar, crearía una tabla con columnas (tipo, ubicación, minAge, maxAge, acción), donde 'acción' es una enumeración que indica cuál de varios tipos de procesamiento que hacer. Idealmente, esta tabla se leería desde un archivo de datos o se mantendría en SQL.

Luego, puede hacer una búsqueda de tabla en el código de Java para determinar la acción que debe realizar un usuario.

-1

Realmente necesita dividir estos casos en métodos de objetos. Supongo que estas cadenas y números se extraen de una base de datos. En lugar de usarlos en su forma cruda en la lógica condicional anidada gigante, necesita usar estos datos para construir objetos que modelen las interacciones deseadas. Considere una clase UserRole con una subclase StudentRole y AdminRole, una clase Region con las subclases de EE. UU. Y México, y una clase AgeGroup con las subclases particionadas apropiadas.

Una vez que tenga esta estructura orientada a objetos en su lugar, podrá hacer uso de patrones de diseño orientados a objetos bien entendidos para volver a factorizar esta lógica.