Yo creo que, lejos de dividir, podemos sumar fuerzas con “Agile Spain” y con cualesquiera otras iniciativas que puedan ayudar a hacer más productivo el sector del desarrollo del software en España.
He enviado un mensaje a los miembros del grupo, si estáis interesados, sólo tenéis que hacer click aquí.
En fin, espero que os animéis, que os unáis a la iniciativa y/o que corráis la voz.
En esta entrega vamos a hacer un cambio parecido al de la entrega anterior (donde transformamos el método Customer.amountFor a Rental.getCharge), pero en este caso moveremos crearemos el método Customer.getFrequentRenterPoints y luego lo moveremos a Rental.
Extracting Frequent Renter Points
Decíamos al final de la entrega anterior que Fowler proponía un último refactor, pero que lo posponíamos. Ahora sí lo hacemos porque veréis que, en mi modesta opinión, es más afín a los siguientes cambios.
1. En Customer.statement sustituimos las ocurrencias de thisAmount por each.getRental(). (En Eclipse, si hacemos doble-click sobre thisAmount veremos que se iluminan todos los puntos donde se usa la variable).
2. Eliminamos la declaración de thisAmount
El código queda como:
while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); // add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints++; // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n"; totalAmount += each.getCharge(); }
Hemos hecho este cambio para así eliminar el uso de thisAmount y ver más claro
el uso que se está haciendo de las demás variables locales. ¡No olvidemos pasar los tests!
Ahora nos fijamos en frequentRenterPoints.
Extraemos el método getFrequentRenterPoints (marcando desde el comentario “add frequent renter points” hasta el comentario “show figures for this rental” y haciendo Alt+Shift+M), pero nos quedará “un poco raro” porque estaremos incrementando la variable y luego asignando su valor en Customer.statement. Así que seguiremos refactorizando hasta tener en Customer.statement la siguiente linea:
frequentRenterPoints += getFrequentRenterPoints(each);
Obsérvese que:
a) quitamos el parámetro frequentRenterPoints de la firma del método getFrequentRenterPoints que nos ha generado Eclipse. Hay que cambiarlo a mano, ahí ya no nos puede ayudar Eclipse. 
b) acumulamos el valor fuera del método (de ahí el “+=”); hay que cambiar el código del método para mantener la integridad semántica. Customer.getFrequentRenterPoints devuelve los valores 1 o 2 (dependiendo de si le corresponde bonus o no a la película que se está alquilando).
c) dado que, no sólo creamos un nuevo método sino que además cambiamos la semántica de las líneas de código originales, es muy importante que pongamos atención al javadoc del método
/** * Returns the renter points corresponding to each rental. A two day new * release rental comes with a bonus. * * @param each * @return */ private int getFrequentRenterPoints(Rental each) { // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) { return 2; } return 1; }
TESTS
Después de este cambio, vemos que Customer.getFrequentRenterPoints no tiene nada
que ver con Customer, por lo que hacemos lo mismo que hicimos con Rental.getCharge
y nos llevamos este método a Rental.
Para ello usamos Alt+Shift+V (pero esta vez desmarcamos la opción de crear el método
delegado), con lo que directamente Eclipse nos cambia la linea anterior a:
frequentRenterPoints += each.getFrequentRenterPoints();
El resultado final de Rental.getFrequentRenterPoints es:
/** * Returns the renter points corresponding to the rental. * A two day new release rental comes with a bonus. * * @param frequentRenterPoints * @return */ int getFrequentRenterPoints() { if ((getMovie().getPriceCode() == Movie.NEW_RELEASE) && getDaysRented() > 1) { return 2; } else { return 1; } }
Observad el matiz del cambio en el javadoc.
TESTS y listo.
Moving the Amount Calculation
Como el método Customer.amountFor (el que hemos creado como consecuencia del refactor de la entrega anterior) usa datos de Rental pero no de Customer, parece que es mejor moverlo a Rental.
Para ello seguimos los siguientes pasos:
1. Si usamos Eclipse, colocamos el cursor sobre el nombre del método y hacemos botón derecho + Refactor / Move. En el diálogo que nos muestra Eclipse:
* Seleccionamos la clase Rental
* Marcamos para que el método actual delegue en el nuevo y que quede marcado como @deprecated
* Podemos ver con Preview cómo van a quedar
2. Ejecutamos los tests para comprobar que los cambios no han estropeado nada.
3. Renombramos el método a Rental.getCharge (y ponemos el javadoc correspondiente). Para renombrar un método, Eclipse proporciona varios caminos: Alt+Shift+ R (el camino corto), Alt+Shift+T + Rename (el camino intermedio) y botón derecho + Refactor + Rename (el camino largo). La ventaja de usar Eclipse para renombrar métodos es que nos va a cambiar el método no sólo en la clase que estamos editando sino también en todas aquellas clases que usen el método (las tengamos abiertas para editar o no).
Hago hincapié en que debemos escribir el javadoc de Rental.getCharge porque cuando cambiamos un método de clase normalmente estamos cambiando la intención del mismo y debemos comunicar esto a los usuarios del método. Para comunicar el cambio usamos normalmente el nombre del método pero no podemos olvidar el comentario javadoc porque puede introducir errores de interpretación si nos dejamos llevar y no los actualizamos (no sólo incluyendo los @deprecated, y cambiando los @param donde toque sino cambiando el texto que explica el comportamiento de los métodos).
4. Volvemos a ejecutar los tests.
A continuación comprobamos que podemos eliminar el método “viejo” (que hemos marcado como @deprecated). (Fowler comenta que en ocasiones podemos preferir dejar el método “viejo” como un delegado al “nuevo” porque, por ejemplo, forme parte de la interfaz pública de la clase).
5. Comprobamos que el método Rental.getCharge sólo es llamado desde Customer.amountFor
y éste sólo desde Customer.statement (con Ctr+Alt+H).
6. Sustituimos la llamada en Customer.statement directamente a Rental.getCharge
thisAmount = each.getCharge();
7. TESTS
8. Comprobamos que Customer.amountFor ya no se usa (con Ctr+Alt+H de nuevo) y lo eliminamos (sin miedo)
9. TESTS
Fowler llama “Move Method” a la refactorización de código que consiste en mover un método de una clase a otra y es el primero de los que propone en el capítulo 7 : “Moving Features Between Objects”. Ahí se explican tanto las motivaciones que nos deberían llevar a realizar este cambio como los pasos a seguir para realizarlo de manera mecánica.
Fowler propone “Replace Temp with Query” en statement para sustituir las ocurrencias
de thisAmount por each.getCharge. (Yo no lo voy a implementar aún).
Recordatorio:
Que no se me olvide, en la entrada anterior “Refactoring en Español (1)” tenéis los enlaces al código con los tests y el código “copi-pegado” del libro.
El primer paso consiste en probar que funcionan los tests. Si alguno no pasa es porque algo tenemos mal configurado.
A continuación, he querido poner comentarios en las clases y el método Customer.statement para darle más claridad a lo que pretendemos hacer.
Seguidamente yo he decidido quitar los warnings en la compilación (estoy usando JDK 6) para lo que he tenido que usar generics en la clase Customer (para dejar bien claro que _rental es una colección de Rental).
Y por fin llegamos al primer refactor de verdad (“Decomposing and Redistributing the Statement Method”). Yo he seguido los siguientes pasos (usando Eclipse como IDE es más fácil porque tiene ciertas ayudas muy útiles):
Decomposing and Redistributing the Statement Method
1. Colocamos el cursor sobre thisAmount en la linea donde se declara esta variable, pulsamos Ctrl+1 y elegimos “Split variable declaration”. De esta manera separamos la declaración de la asignación.
2. Movemos la asignación (thisAmount = 0) hasta después del comentario que dice:
// determine amounts for each line
3. Extraemos el método amountFor desde el comentario hasta el final del switch. Para ello seleccionamos todo ese bloque de código (incluido el comentario), pulsamos Alt+Shift+M (también podemos hacer Ctrl+1 + “Extract method”) y elegimos el nombre amountFor para el nuevo método.
4. En el método amountFor hacemos Ctrl+1 + “Join split declaration” para thisAmount.
5. Idem en el método statement (también para thisAmount).
6. Movemos el comentario “inline” al javadoc del método amountFor.
7. Cambiamos el nombre del atributo each a aRental (usando Ctrl+2).
Lo correcto es ejecutar los tests después de cada paso (si movemos comentarios, no, claro). Nos puede parecer una tontería ejecutar los tests después de un cambio tan “tonto” como los que se hacen en 4 ó 5, pero se trata de una técnica que se basa en realizar avances pequeños pero siempre con seguridad. Esta técnica nos dice que, si en algún momento pasamos de “verde a rojo” (de pasar todos los tests a que alguno falle), deshagamos los cambios y volvamos a realizarlos. SIN CONTEMPLACIONES. El fundamento de esto es claro: si hacemos un cambio sobre un código que está “en rojo” corremos el riesgo de no arreglar lo que falla e introducir nuevos errores. Por ello, lo mejor es volver “a terreno seguro” y replantearse el cambio de nuevo.
Bueno, en esta entrega hemos visto el refactor más sencillo (Extract Method) y en la siguiente entrega veremos un paso más en el ejemplo de Fowler (“Moving the Amount Calculation”) donde veremos otro refactor más complicado (Move Method) y seguiremos comprobando cómo Eclipse nos ayuda en estas tareas que, para hacerlas bien, hay que ser muy meticuloso.
Corrección:
Estaba preparando la siguiente entrega de esta serie y me he dado cuenta de que no había incluido el enlace al código y a los tests.
