Script Refactoring

¿Qué es refactorizar?

  • Es cambiar la estructura del código para hacerlo más simple y extensible. Esto involucra muchas acciones posibles, algunas simples como cambiar el nombre de una clase o un método, o algo más complejo como generar interfaces, subclasificar comportamientos, generar nuevos objetos o bien separar una clase en varias.
  • Refactoring vs. incorporar funciones nuevas: Ojo, no significa agregar funcionalidad, si bien el refactor suele surgir cuando hay que incorporar cosas nuevas a una aplicación. Debemos separar claramente los dos momentos: en uno nuestro objetivo es simplificar el código (refactoring), en otro aprovechar ese código simplificado para poder agregar funcionalidad (expansión).
  • Refactorizar vs. optimizar: son cosas distintas porque tienen objetivos distintos. En un desarrollo lo aconsejable es refactorizar primero y optimizar sólo cuando necesite (y no siempre necesito pensar en performance). Cuando refactorizo el código queda más claro y mantenible, cuando optimizo en general es al revés.

Estamos hablando de Refactoring en OO. ¿Por qué?

Porque estamos viendo Diseño OO, claro. Pero además…

1. La IDE de los lenguajes OO colabora mucho en esta tarea, de hecho viene con menúes Refactor, mientras que las IDE de los lenguajes híbridos recién ahora están incorporando estas herramientas. Algunas cosas que damos por sentadas pero que conviene valorarlas son:

  • pretty print: colorear lo que son palabras reservadas, comentarios, argumentos, variables, etc. es importante
  • formatters: no dejar que cada programador indente a mano con espacios / tabs, sino que eso sea parte de la configuración del equipo y que la IDE lo haga solo
  • manejo de dependencias: tanto a nivel clase (imports) como a nivel proyecto (utilidades y plugins para localizar jars)

son tareas básicas que influyen en el refactor (tener un método con algunas líneas comentadas y verlas todas en blanco y negro, o bien métodos con diferente indentación agrega más cosas en las cuales pensar a la hora de mejorar lo que está escrito)

2. (Porque nos dijeron que en OO es más fácil reutilizar código)
Claro, eso requiere de codificar siguiendo buenas prácticas:

  • Una consiste en programar pensando en la interfaz (qué necesito/qué ofrezco) y menos en la implementación (si lo resuelvo con una colección de tipo List o Set, o con un flag booleano o un strategy, etc.)
  • Otra es minimizar el acoplamiento entre objetos (no conocer cosas de más)
  • Otra es subir la cohesión de los métodos y las clases: tener métodos cortos que tengan un objetivo claro y definido, y que eso me lleve a entender cuál es el objetivo principal de la clase, si tengo muchos objetivos puedo perder foco en lo que quiero hacer
  • Y por supuesto, el polimorfismo es una herramienta esencial para poder enviar mensajes a objetos intercambiables
  • La herencia, en menor medida, también colabora en la reutilización al permitir extender comportamiento existente de las superclases.

3. Es imprescindible (sí, imprescindible) contar con una herramienta de testeo unitario, para garantizar que el refactoring no alteró el comportamiento del sistema. Mientras mejores tests tengamos, tanto más seguridad podemos brindar de que el refactor cumplió su objetivo. Sin herramientas de testeo unitario el miedo se apodera de los programadores, un miedo que en este caso es sano y promueve la supervivencia del sistema, pero que a su vez paraliza las posibilidades de mejorar el código ("no tocar lo que anda", que en cierta bibliografía podrán estudiar como "Línea de defensa Maginot", negar la posibilidad de cambiar).

¿Qué busca el refactor? ¿Cómo es "más simple"?

  • Métodos cortos y con nombres bien definidos (intention revealing, que revelen exactamente el propósito para el que fueron creados)
  • Métodos y clases con responsabilidades claras y bien definidas (mayor cohesión)
  • Pocas variables de instancia: hay que evaluar el costo-beneficio de tener en variables valores que puedan calcularse, como el total de deuda de un cliente, o la cantidad de hijos de un empleado
  • No hay "god objects" ni "managers"
  • Tampoco hay objetos anémicos, con pocas responsabilidades que se parecen a estructuras de datos
  • Es preferible tener más objetos chicos que un objeto grande con muchas responsabilidades (sobre todo si los objetos chicos pueden intercambiarse)
  • Es preferible abstraer muchas interfaces que tener una sola interfaz con poca cohesión y mucho que hacer (un objeto puede ser Serializable y no Comparable). Esto tiene un nombre: "The Interface Segregation Principle" por si les gusta googlear.
  • Evitar ciclos de dependencia entre objetos si no los necesito. Ej: cada cliente conoce a sus facturas, la factura ¿necesita conocer al cliente? muchas veces, por las dudas, tenemos relaciones innecesarias. También hay un nombre para eso: "The Acyclic Dependencies Principle". Como dice Opdyke: "One asymmetric aspect of an aggregation is that the aggregate object usually needs to know about its components, but less often does a component need to know either about the aggregate object that contains it, or about other components."

Code smells: "if it stinks, change it"

O sea, si huele mal hay que cambiarlo, consejo de la abuela de Kent Beck sobre los pañales que es aplicable también al código.
Los smells son señales de que hay algo en el código que no anda bien, que podemos mejorar. Algunos ejemplos:

Code smells ("olores")

Misplaced methods

Ejemplo: tenemos una aplicación para Papá Noel donde debemos calcular la cantidad de maldad de una acción. En la clase PapaNoel codificamos:

public double getCantidadMaldad(Accion accion) {
   return accion.getCantidadMaldad();
}

Si no accedemos a ninguna variable ni tampoco utilizamos comportamiento interno, posiblemente estamos ubicando mal el método.

Duplicated Code

Missing inheritance or delegation

Esto lo vamos a desarrollar en breve, pero es parte del objetivo de la materia tener herramientas para no duplicar la misma idea
en el código. Dos lemas que nos acompañan son:

  • Once and only once: hacer las cosas una sola vez
  • Don't repeat yourself (DRY)

y esto no sólo vale para el código, también aplica para el diseño.

Long Method

Inadequate decomposition

Un método largo podemos descomponerlo en varias partes.

  • cada parte es una oportunidad para generar un método (con un nombre representativo)
  • los métodos generados pueden ser utilizados en otro contexto
  • delegar el método original en varios submétodos nos permite entender mejor qué es lo que hace

Esta técnica no es excluyente del paradigma de objetos, de hecho es la técnica de descomposición por procesos que predican los lenguajes imperativos.

Large Class / God Class

Too many responsibilities

Visto en varias oportunidades dentro de la materia, una clase con muchas responsabilidades conoce información de muchos objetos, y se ve rápidamente impactada ante cualquier cambio. Cualquier agregado o corrección de estas clases suele representar un dolor de cabeza. La solución no es trivial, dado que requiere repensar el diseño (eso implica poner responsabilidades en distintas clases, o crear clases nuevas).

Long Parameter List

Object is missing

Cuando un objeto ofrece un servicio hacia los demás, los publica a partir de una interfaz. Y esa interfaz está bueno cambiarla lo menos posible.
La interfaz de un método está dado por:

  • si lo define el lenguaje, la visibilidad
  • lo que devuelve (en lenguajes con chequeo estático incluye el tipo de lo que devuelve o void)
  • el nombre del método
  • los parámetros (nuevamente, si el lenguaje tiene chequeo estático también son importantes los tipos de esos parámetros)
  • y no olvidemos las excepciones chequeadas

Entonces, tener un método en Cliente

public List getVentas(Date fechaDesde, Date fechaHasta, boolean incluyeVentasPendientes, String nombreVendedorLike, String productoEmpiezaCon, BigDecimal montoMinimo, BigDecimal montoMaximo, ...) {
    ...
}

es muy probable que sufra cambios el día de mañana, porque estamos pasando una estructura bastante importante como parámetro.
La solución: generar un objeto que cumpla esta misión, en principio como agrupador de los parámetros de búsqueda:
public List getVentas(BusquedaVentas busqueda) {
    ...
}

Esto permite ampliar, modificar o quitar parámetros de búsqueda sin afectar la interfaz del objeto.

Type Tests

Missing polymorphism

Preguntarle a los objetos "de qué tipo sos" es señal de no haber utilizado correctamente objetos polimórficos.

Message Chains

Coupled classes, internal representation dependencies

Cuando uno ve que un objeto envía un mensaje de la forma:

a. b() . c() . d()

(ya sea directamente o a través de llamadas intermedias)

Va en contra del consejo Tell, don't ask y de la Ley de Demeter ("don't talk to strangers", "only talk to friends"), donde un objeto sólo debería enviar mensajes:

  • a sí mismo
  • a objetos que conoce (como variables de instancia)
  • a objetos que recibe como parámetro
  • a objetos que instancia

De todas maneras, la ley de Demeter no siempre puede cumplirse a rajatabla y hay que tener cuidado con llevarlo a un pensamiento purista y dogmático (sobre todo con el manejo de colecciones).

Data Clumps

Data always used together

Ejemplos:

  • x,y está representando un objeto Point
  • calle, altura, piso, departamento, etc. forman parte de un objeto Direccion
  • dni, nombre puede representar a una Persona

Si estamos trabajando un conjunto de datos que está relacionado nos estamos perdiendo una abstracción: es importante reconocerla,
darle un nombre y sobre todo, una entidad. Eso nos permite asignarle responsabilidades, y vuelve al sistema menos permeable a los
cambios. Refactor asociado: Extract class

Temporary Field

Attributes only used partially under certain circumstances

Una clase Factura que tiene un variable privada double total, otra totalConIVA y otra totalSinIVA, o un Cliente que tiene una variable privada ultimoPago cuando también tiene una colección de pagos. Es decir que "cachea" valores que podría calcular. De todas maneras esto no constituye un problema per se, uno puede tomar la decisión contraria basándose en:

  • que es costoso calcular el valor y se necesita conocer ese valor muchas más veces que las veces que se actualiza
  • temas de implementación (como la necesidad de guardar ese valor en una base de datos relacional para luego ejecutar queries de consulta, así estaríamos evitando duplicar la lógica de negocio en el SQL, pero como vemos es un caso que escapa al contenido de la materia)

Data Classes

Only accessors

Una clase que sólo representa una estructura de datos, es fácil reconocerla cuando tiene sólo getters y setters. Los DTO, los POJO, los Entity Bean 2.0 entran en esta categoría. Un contraejemplo son los Value Models, u objetos que modelan los parámetros que enviamos a un objeto (relacionado con Long Parameter List). Pero sí constituye una mala práctica pensar que es bueno separar un objeto en

  • atributos
  • y comportamiento

de manera de tener un ClienteDTO o ClientePOJO (sólo estructura de datos + getters y setters) y luego varios objetos que modelen "procesos" que aplican a esos objetos ClientesDTO.
¿Por qué es una mala práctica? Porque

  • descree del principio del paradigma (objeto agrupa atributos y comportamiento)
  • los objetos resultantes tienen muy poca cohesión y un alto acoplamiento entre sí (es fácil darse cuenta de que cualquier cambio en la estructura de datos causa un inmediato impacto en los objetos proceso)
  • el corolario de lo expuesto anteriormente es que no hay encapsulamiento: la implementación de cada atributo está expuesta en los getters y setters del objeto DTO
  • es difícil trabajar con clientes polimórficos: el comportamiento está puesto fuera del objeto que representa al cliente
  • es difícil no repetir ideas de código si tenemos varios objetos que representen funcionalidades del cliente (aplicar descuentos, calcular deuda, facturar, etc.)

Primitive Obsession

Representar con ints, booleans, Strings o enumeraciones cosas que podrían ser objetos con comportamiento. Las enumeraciones nos llevan a tener sentencias condicionales en lugar de trabajar con objetos polimórficos. O bien preferir el Array [] en lugar de tener objetos que modelen una colección, basta con ver la rica interfaz de Collection en contraposición a lo poco que ofrece el Array.

Refused Bequest

Utilizar herencia cuando se puede obtener lo mismo con delegación. Ejemplo típico: ¿una Pila hereda de List o tiene un List?
La herencia es rígida:

  • obliga a definir más métodos que el necesario (la Pila tiene que implementar o bien heredar containsAll, clear y muchos métodos que quizás no apliquen para el concepto en sí)
  • en la mayoría de las implementaciones, sólo puedo heredar una vez, eso es una limitante (permite un solo punto de vista)

Inappropriate Intimacy (Subclass Form)

Cuando una subclase accede directamente a las variables de su superclase en lugar de utilizar los getters (acceso indirecto). Este bad smell es el corolario de la frase "la herencia viola el encapsulamiento", o bien de que la herencia me lleva a un acoplamiento mayor que la delegación, por eso el consejo del libro de Design Patterns "Favor object composition over class inheritance". De todas maneras, no es grave suponer que cuando uno modifica una superclase es muy probable que las subclases se vean afectadas.

Lazy Class

Suele ocurrir al sobrediseñar jerarquías para uso futuro: armamos la interfaz IModel, tenemos luego una clase abstracta
AbstractIModel con una implementación default: DefaultModel concreta que hereda de AbstractIModel. Pero la clase AbstractIModel
no define comportamiento ni atributos, está PLD (por las dudas).

En ese caso el consejo de los extreme programmers "no programar para el cambio" resulta útil: pensar en lo más simple nos llevaría
a primero definir DefaultModel, luego encontrar como abstracción la interfaz IModel y finalmente, cuando necesitamos nuevas
implementaciones de DefaultModel pero que tengan alguna especialización, nos abocamos a crear una superclase (pull-up) para poner
el comportamiento común.

Parecido a esto es el acrónimo YAGNI "You aren't gonna need it": no agregar funcionalidad hasta que sea necesario (TDD ayuda para que esto no pase)

Feature Envy

Method needing too much information from another object

Esto ocurre cuando una clase A envía demasiados mensajes a otra clase B, quizás porque la clase B no esté ofreciendo el servicio que la clase A necesita.
Un ejemplo extraído de http://c2.com/cgi/wiki?ManifestResponsibility es cuando tenemos una clase Carta, que conoce a una persona y definimos un método

public String getDestinatario() {
   return persona.getNombre() + " " + persona.getDireccion() + " " + persona.getEdad();
}

La clase Carta está pidiendo demasiada info a la persona, entonces debería ser responsabilidad de la persona devolver el domicilio completo:

>>Carta
public String getDestinatario() {
   return persona.toString();
}

>>Persona
public String toString() {
   return this.getNombre() + " " + this.getDireccion() + " " + this.getEdad();
}

Middle Man

Class with too many delegating methods

public class MiddleMan {
    f( ) {
        delegate.f(); 
    }

    g( ) {
        delegate.g(); 
    }

    h( ) {
        delegate.h(); 
    }
}

Una clase "recepcionista" que en realidad termina delegando a otro objeto toda la responsabilidad.
Suele verse en Facades donde en realidad hay uno o muy pocos objetos a los cuales se delega y poco valor agregado del Facade (simplemente redespachar el mensaje).
Por otra parte, también puede ocurrir al tratar de evitar Message Chains (ver más abajo), ejemplo:

a.b().c().d()

En lugar de eso, hacemos:

    ...
    a.d()
    ...

>>Clase A
public void d() {
    this.b().d();
}

>>Clase B
public void d() {
    this.c().d();
}

>>Clase C
public void d() {
    ... implementación final...
}

Alguna de las clases puede no tener demasiado comportamiento y quedar simplemente como un intermediario de las otras clases.

El problema del Middle Man es que la clase intermediaria no juega ningún papel, sólo sirve como pasamanos: too much simple delegation instead of really contributing to the application.

Divergent Change

Same class changes differently depending on addition

Este bad smell ocurre cuando tenemos una clase que tiene más de un objetivo:

  • objetivo 1: 2 atributos y 3 métodos
  • objetivo 2: 3 atributos y 4 métodos

Pero esos atributos y métodos son disjuntos entre cada objetivo. Entonces lo que está ocurriendo es que deberíamos separar esa clase en dos, porque si no cada modificación me llevará a actualizar los métodos y atributos del objetivo 1 ó del 2. La solución está en el refactor Extract class.

Shotgun Surgery

Small changes affect too many objects

Este ejemplo de Nick Harrison extraído de http://www.simple-talk.com/dotnet/.net-framework/exploring-smelly-code/
ilustra lo que sucede cuando nos toca recuperar información de una base de datos. Entonces generamos el siguiente método:

public ResultSet getClientes() {
    SqlConnection con = new SqlConnection(this.connectionString);
    SqlCommand cmd = con.createCommand();
    con.open();
    cmd.commandType = CommandType.StoredProcedure;
    cmd.commandText = "spGetClientes";
    return cmd.getResults(CommandBehavior.CloseConnection);
}

(el código es Java-C#-like)

Lo mismo suele pasar con

  • temas de persistencia
  • manejo de errores: ¿qué queremos hacer cada vez que ocurra un error?
  • activar o desactivar niveles de debug/logging para medir performance/atacar errores
  • manejo de configuraciones de una aplicación
  • en general donde necesitamos hacer algo en muchos lugares, ese algo involucra varios pasos (al menos dos) y queremos tener un comportamiento uniforme.

En esos casos, lo ideal es concentrar en un único punto esos pasos para que cuando nos pidan un cambio aparentemente pequeño eso no nos impacte en una gran cantidad de objetos.

Refactor que permite resolver este bad smell: Extract Class + Move Method

public ResultSet getClientes() {
    new StoredProcedure().getResults("spGetClientes");
}

>>StoredProcedure (clase creada por nosotros)
public StoredProcedure() {
    SqlConnection con = new SqlConnection(this.connectionString);
}

public ResultSet getResults(String storedProc) {
    SqlCommand cmd = con.createCommand();
    con.open();
    cmd.commandType = CommandType.StoredProcedure;
    cmd.commandText = storedProc;
    return cmd.getResults(CommandBehavior.CloseConnection);
}

¿Cómo hacer refactoring? Refactors más comunes

Comentarios

Cualquier comercial de Sprayette podría incluir deshacerse del código comentado que lleva años juntando polvo.
Código que uno lee release tras release y que sólo deja la sensación de que lo comentado algún día servirá para arreglar un problema que todavía no apareció.

Falta de polimorfismo

  • "Tell, don't ask"

pasamos de sentencias case (if múltiples)

   ...
   if (this.tipoEmpleado.equalsIgnoreCase("E")) {
      return this.empleado.getPorcentajeEfectivo(...);
   } 
   if (this.tipoAccion.equalsIgnoreCase("J")) {
      return this.empleado.getPorcentajeJerarquico(...);
   } 
   if (this.tipoAccion.equalsIgnoreCase("I")) {
      return this.empleado.getPorcentajeIndependiente(...);
   } 
   ...

a delegar en objetos que comparten la misma interfaz
   ...
   return this.empleado.getPorcentaje(...);
   ...

Las implementaciones del getPorcentaje(…) puede corresponder a subclases de Empleado (Efectivo, Jerarquico e Independiente)
o bien a strategies Efectivo, Jerarquico e Independiente de tipo de empleado.
  • State/Strategy/Null Object patterns evita preguntas

En el State, estados que derivan en distintos comportamientos se vuelven polimórficos.
En el Strategy, los algoritmos se intercambian.
En el Null Object el que usa objetos posta o nulos no quiere distinguir ese caso particular, por eso se beneficia del polimorfismo
para evitar

   ...
   if (this.seleccionado != null) {
      ...
   } else {
      ...
   }

Código duplicado

  • Extract Method dentro de la misma clase.

Ej: Papá Noel de la clase 1

    public double getCantidadMaldad() {
           if (this.seArrepintio) {
              return this.getGravedad() * 0.5 * this.getCantidadAfectados();
           } else {
              return this.getGravedad() * this.getCantidadAfectados();
           }
    }

Si proponemos llevar afuera la pregunta de si se arrepintió, nos queda:
    public double getCantidadMaldad() {
           return this.getGravedad() * this.getCantidadAfectados() * this.getCoeficienteAjuste();
    }

    public double getCoeficienteAjuste() {
       if (this.seArrepintio) {
          return 0.5;
       } else {
          return 1;
       }
    }

Se elimina el código duplicado (multiplicación de cantidad de afectados por gravedad)
  • Entre clases hermanas se puede extraer el comportamiento y ubicarlo en una superclase común a ambas

Ejemplo: Las acciones buenas y las malas tienen personas que se ven afectadas por dicha acción, esto se
implementa con una variable de instancia

Collection<Persona> afectadas;

en AccionBuena y AccionMala respectivamente.
Para filtrar las personas malas afectadas disponemos de un método:
    public List<Persona> getAfectadosMalos() {
       List<Persona> malos = new ArrayList<Persona>();
       for (Persona afectado : this.getAfectados()) {
          if (!afectado.esBueno()) {
             malos.add(afectado);
          } 
       }
       return malos;
    }

El mismo código está en AccionMala.
Podemos pararnos sobre el método, botón derecho: Refactor > Pull-up y subirlo a la clase Accion.
  • Si tenemos dos clases hermanas cuyo comportamiento es el mismo, pero una tiene una ligera especialización, podemos poner el comportamiento común en la superclase y refinar la especialización delegando a su vez en el método que está en la superclase. Ejemplo: el volumen que necesitan los productos para ser almacenados se mide en base al alto, ancho y largo. El volumen que necesitan los productos especiales es el doble de los productos comunes. Tenemos la clase Producto con el siguiente método:
public double getVolumen() {
    return this.getAlto() + this.getAncho() + this.getLargo();
}

Para evitar que ProductoEspecial haga lo mismo pero multiplicado por dos podemos utilizar super:
public double getVolumen() {
    return super.getVolumen() * 2;
}

Esto delega el comportamiento default a la superclase y permite no verse impactado por un cambio en el cálculo del volumen default (puedo modificar las tres variables por una variable volumen en metros cúbicos sin tener que tocar la subclase).
  • También el template method funciona como un mecanismo para evitar duplicación de código. En el mismo ejemplo del volumen, podemos tener una clase abstracta Producto con un método default que sea getVolumen(), implementado de la siguiente manera:
>>Producto (abstracta)
public double getVolumenBase() {
    return this.getAlto() + this.getAncho() + this.getLargo();
}

public double getVolumen() {
    return this.getCoeficienteAjuste(this.getVolumenBase());
}

public abstract double getCoeficienteAjuste(double volumenBase);

>>ProductoComun
public double getCoeficienteAjuste(double volumenBase) {
    return volumenBase;
}

>>ProductoEspecial
public double getCoeficienteAjuste(double volumenBase) {
    return volumenBase * 2;
}

(el lector puede buscar soluciones alternativas)
  • Entre clases no relacionadas, o bien podemos crear una superclase en común, o bien crear un componente nuevo y referenciar a él (un Helper, un Util o bien un objeto al que tenga sentido darle un nombre)

Métodos largos

  • Descomponerlos en métodos más pequeños (tip: aprovechar los comentarios para delimitar cuándo comienza y cuándo termina un método). Encontrar objetos "ocultos", o bien asignarles responsabilidades que no tienen. En general, preferimos componer antes que subclasificar, pero ambas herramientas son útiles para no tener métodos largos.

Refactors que propone el IDE

Rename (Alt + Shift + R)

Para recalcar: el IDE presupone una revolución en una tarea tan simple como renombrar una clase, una variable o un método.
Porque cuando trabajamos con editores de texto planos, la opción equivalente (Find and Replace All) tenemos que hacerlo en
forma selectiva y cuidadosa. Ejemplo: tenemos un objeto Empleado con una variable for931 y nos damos cuenta de que podríamos
mejorar la expresividad de la variable cambiándole el nombre a formulario931. Entonces pedimos un Find & Replace de for a formulario.
El resultado puede ser bastante decepcionante: todos los loops for aparecen ahora:

    formulario (int i = 0; i < MAX_LENGTH; i++) {
       // Used formulario special purposes
       ...
    }

Y además hay que considerar:

  • al renombrar variables pueden terminar renombrando nombres de los métodos
  • también hay que distinguir variables locales de las de instancia
  • al renombrar métodos pueden afectar a partes del código
  • y también hay que tener en cuenta las referencias a ese método (superclases o interfaces, objetos que envían mensajes con dicho nombre, etc.)

Contar con un IDE permite evitar estas molestas confusiones, y reduce el lógico miedo a cambiar lo que está funcionando.

Move (Alt + Shift + V)

De la mano con el Rename, el Move de clases hacia otros packages, o de métodos hacia otras clases es fundamental hacerlo desde una IDE.

Extract Local Variable (Alt + Shift + L)

    /**
     * 1.a Saber la cantidad de maldad de una acción
     */    
    @Override
    public double getCantidadMaldad() {
        if (this.seArrepintio) {
            return 0.5 * this.gravedad * this.getCantidadAfectados();
        } else {
            return this.gravedad * this.getCantidadAfectados();
        }
    }

[[/code]]
Marcamos this.gravedad * this.getCantidadAfectados() con Alt + Shift + L a "maldadDefault" y nos queda:

    /**
     * 1.a Saber la cantidad de maldad de una acción
     */    
    @Override
    public double getCantidadMaldad() {
        double maldadDefault = this.gravedad * this.getCantidadAfectados();
        if (this.seArrepintio) {
            return 0.5 * maldadDefault;
        } else {
            return maldadDefault;
        }
    }

Extract Constant

Podemos reemplazar el 0.5 por una constante: esto nos permite pasar de

    /**
     * 1.a Saber la cantidad de maldad de una acción
     */    
    @Override
    public double getCantidadMaldad() {
        double maldadDefault = this.gravedad * this.getCantidadAfectados();
        if (this.seArrepintio) {
            return 0.5 * maldadDefault;
        } else {
            return maldadDefault;
        }
    }

a…
    private static final double PORCENTAJE_ARREPENTIMIENTO = 0.5;

    /**
     * 1.a Saber la cantidad de maldad de una acción
     */    
    @Override
    public double getCantidadMaldad() {
        double maldadDefault = this.gravedad * this.getCantidadAfectados();
        if (this.seArrepintio) {
            return PORCENTAJE_ARREPENTIMIENTO * maldadDefault;
        } else {
            return maldadDefault;
        }
    }

Extract Method (Alt + Shift + M)

Otra opción para refactorizar la cantidad de maldad:

    @Override
    public double getCantidadMaldad() {
        if (this.seArrepintio) {
            return PORCENTAJE_ARREPENTIMIENTO * this.gravedad * this.getCantidadAfectados();
        } else {
            return this.gravedad * this.getCantidadAfectados();
        }
    }

Marcamos después del return el this.gravedad * this.getCantidadAfectados()
Hacemos Alt + Shift + M y extraemos un método getMaldadDefault:
    @Override
    public double getCantidadMaldad() {
        if (this.seArrepintio) {
            return PORCENTAJE_ARREPENTIMIENTO * this.gravedad * this.getCantidadAfectados();
        } else {
            return getMaldadDefault();
        }
    }

    private double getMaldadDefault() {
        return this.gravedad * this.getCantidadAfectados();
    }

Claro, el primer this.gravedad * this.getCantidadAfectados() no lo reconoce como en el extract variable pero es cuestión
de modificarlo.

Change Method Signature (Alt + Shift + C)

Para modificar la interfaz de un método, es una buena opción utilizar el refactor de la IDE (agregar o sacar parámetros o
cambiarles el tipo).
Ejemplo: en el método esBueno podemos agregar el año como parámetro

    public boolean esBueno() {
        int anioActual = Calendar.getInstance().get(Calendar.YEAR);
        int anioAnterior = anioActual - 1;
        return this.getNivelBondad(anioActual) > 0 || this.getNivelBondad(anioAnterior) < this.getNivelBondad(anioActual);
    }

Marcamos el nombre esBueno y Alt + Shift + C:

En Parameters incorporamos una línea más:
Type: int, name: anio, Default value: Calendar.getInstance().get(Calendar.YEAR);

Comparemos con una modificación manual: es muchísimo más feliz ver que en cada llamada al método esBueno se agrega el valor default
al parámetro adicional:

  • getCantidadBondad() de Bondad
    @Override
    public double getCantidadBondad() {
        double total = 0;
        for (Persona afectado : this.getAfectados()) {
            if (!afectado.esBueno(Calendar.getInstance().get(Calendar.YEAR))) {
                total += this.importancia * PORCENTAJE_BUEN_SAMARITANO;
            } else {
                total += this.importancia;
            }
        }
        return total;
    }
  • getChicosBuenos() de Taller
  • fernanditoEsBueno() de TestPapaNoel
  • germancitoEsMalo() de TestPapaNoel

Sólo nos queda modificar a mano el método original:

    public boolean esBueno(int anio) {
        int anioAnterior = anio - 1;
        return this.getNivelBondad(anio) > 0 || this.getNivelBondad(anioAnterior) < this.getNivelBondad(anio);
    }

Incluso podríamos guardar en una variable temporal (Alt + Shift + L) el nivel de bondad del año actual:

    public boolean esBueno(int anio) {
        int anioAnterior = anio - 1;
        double nivelBondadAnioActual = this.getNivelBondad(anio);
        return nivelBondadAnioActual > 0 || this.getNivelBondad(anioAnterior) < nivelBondadAnioActual;
    }

Pero eso va en gustos…

Extract Superclass

Parecida a pull up, sólo que no existe la superclase que queremos crear

Inline (Alt + Shift + I)

Hace desaparecer variables locales y las reemplaza por envío de mensajes

Extract Interface

Pull up / Push Down

Extract class

Introduce Parameter Object (resuelve el Long Parameter List)

Si tenemos un método con muchos parámetros, del estilo:

public void generarFacturacion(Periodo periodo, TipoCliente tipoCliente, Zona zonaFacturacion, int coeficienteAjusteInflacionario) {
    ...
}

Nos posicionamos en la línea donde se define el método y con botón derecho Refactor > Introduce Parameter Object nos ofrece crear una clase nueva llamada GenerarFacturacionParameter que conoce a un Periodo, un TipoCliente, una Zona y un int como coeficienteAjusteInflacionario). Podemos modificar el nombre de la clase en la ventana de diálogo y en esa misma ventana pedirle que nos cree los getters y los setters.

Limitaciones del Refactoring

  • External systems that can't be refactored: if you are using a external database or large code framework, those can't be refactored, limiting the improvements that can be made. (Note however that with shared code you can refactor your team-mates code; this is one of the main strengths of having a shared codebase)
  • Interfaces that are published should change as little as possible, but refactoring often wants to change those interfaces. There is a significant tension here.
  • If the design is too awful, its not worth trying to refactor—-throw it out!

Y después del refactoring, qué

Método MoSCoW para saber en qué orden agregamos las funcionalidades:

  • Must
  • Should
  • Could
  • Would

También se manejan con prioridades, o bien se generan iteraciones para ir haciéndolos aparecer.

Ojo con la pasión por las métricas: está bueno cuando una herramienta automatizada nos ayuda a detectar posibles bad smells de código. No volvernos fanáticos de las herramientas.
En definitiva, no tomar al libro de Refactoring como una biblia, es una herramienta más, como el TDD, que viene incorporada en muchas IDEs y nos ayuda a pensar en el diseño, a evitar ideas repetidas y a mitigar la entropía de un sistema.

Unless otherwise stated, the content of this page is licensed under Creative Commons Attribution-ShareAlike 3.0 License