En esta entrega vamos a ver cómo nos llevamos la lógica condicional que hay en los métodos Rental.getCharge y Rental.getFrequentRenterPoints a la clase Movie.

Replacing the Conditional Logic on Price Code with Polymorphism

Fowler afirma que es una mala idea basar un switch en un atributo que corresponde a otro objeto. Dado que se basa en un atributo de Movie, el método debería estar en Movie (aunque tengamos que pasar el número de días de alquiler como parámetro).

Los pasos a seguir son los siguientes (suponiendo que tenemos Eclipse):
1. Alt+Shift+V sobre el método Rental.getCharge

• Seleccionamos Movie
• Marcamos la opción de crear un método delegado (pero sin marcarlo como @deprecated)
• Revisamos el javadoc porque ahora el método está en otro objeto y puede tener otras responsabilidades.

Con esto, habremos dejado el código como sigue:

 /**
  * @see Movie#getCharge(int)
  */
 double getCharge() {
  return _movie.getCharge(getDaysRented());
 }

 /**
  * Determine the corresponding amount depending on the type of movie
  * (the price code).
  *
  * @param rental TODO
  * @return   */
 double getCharge(Rental rental) {
  double thisAmount = 0;
  switch (rental.getMovie().getPriceCode()) {
    case Movie.REGULAR:
      thisAmount += 2;
      if (rental.getDaysRented() > 2)
        thisAmount += (rental.getDaysRented() - 2) * 1.5;
      break;
    case Movie.NEW_RELEASE:
      thisAmount += rental.getDaysRented() * 3;
      break;
    case Movie.CHILDRENS:
      thisAmount += 1.5;
      if (rental.getDaysRented() > 3)
        thisAmount += (rental.getDaysRented() - 3) * 1.5;
      break;
   }
   return thisAmount;
 }

TESTS

2. El atributo sobre el que se hace el switch es equivalente a getPriceCode() puesto que se trata del mismo objeto Movie; así que esa linea pasa de:

  switch (rental.getMovie().getPriceCode()) {

a:

  switch (getPriceCode()) {

TESTS

3. Cambiamos el atributo que pasamos para que, en vez de pasar el objeto Rental, pasar el número de días que se alquila.

• Hay que cambiar todas las rental.getDaysRented por el parámetro daysRented
• Cambiamos el javadoc porque ahora el método recibe parámetros distintos.

El código de este método queda como sigue:

 /**
  * Determine the corresponding amount depending on the type of movie
  * (the price code) and the number or days rented.
  *
  * @param daysRented
  * @return   */
 double getCharge(int daysRented) {
  double thisAmount = 0;
  switch (getPriceCode()) {
    case Movie.REGULAR:
      thisAmount += 2;
      if (daysRented > 2)
        thisAmount += (daysRented - 2) * 1.5;
      break;
    case Movie.NEW_RELEASE:
      thisAmount += daysRented * 3;
      break;
    case Movie.CHILDRENS:
      thisAmount += 1.5;
      if (daysRented > 3)
        thisAmount += (daysRented - 3) * 1.5;
      break;
    }
    return thisAmount;
 }

TESTS

Por simetría (un importante patrón, ver “Implementation Patterns” de Beck), implementamos el método Rental.getFrequentRenterPoints en Movie (pasando el número de días de alquiler como parámetro).

Lo dejo como ejercicio porque es simplemente aplicar la misma receta sobre el método Rental.getFrequentRenterPoints llevando el código al nuevo Rental.getFrequentRenterPoints(days:int).

TESTS

Llegados a este punto… me doy cuenta de que no estamos usando TDD. Es decir, que los tests que tengo no están probando el código que estoy escribiendo. Me refiero a que hemos movido código entre clases y no hemos escrito tests para estas clases.

He escrito MovieTest.

package step5;

import static org.junit.Assert.assertEquals;
import org.junit.Ignore;
import org.junit.Test;

public class MovieTest {
 private static final String ANY_TITLE = "Any Movie Title";
 private static final int UNKNOWN_PRICE_CODE = 99;
 @Test  public void testMovieRegular() {
  Movie aMovie = new Movie(ANY_TITLE,Movie.REGULAR);
  assertEquals(Movie.REGULAR, aMovie.getPriceCode());
 }

 @Test  public void testMovieChildren() {
  Movie aMovie = new Movie(ANY_TITLE,Movie.CHILDRENS);
  assertEquals(Movie.CHILDRENS, aMovie.getPriceCode());
 }

 @Test  public void testMovieNewRelease() {
  Movie aMovie = new Movie(ANY_TITLE,Movie.NEW_RELEASE);
  assertEquals(Movie.NEW_RELEASE, aMovie.getPriceCode());
 }

 @Test(expected=IllegalArgumentException.class)
 @Ignore  public void testMovieIllegalType() {
  new Movie(ANY_TITLE,UNKNOWN_PRICE_CODE);
 }

 @Test  public void testFrequentRenterPointsRegular() {
  int points[] = { 1, 1, 1 };
  Movie aMovie = new Movie(ANY_TITLE,Movie.REGULAR);
  for (int daysRented = 1; daysRented <= points.length; daysRented++) {
   assertEquals(points[daysRented-1], aMovie.getFrequentRenterPoints(daysRented));
  }
 }

 @Test  public void testFrequentRenterPointsChildrens() {
  int points[] = { 1, 1, 1 };
  Movie aMovie = new Movie(ANY_TITLE,Movie.CHILDRENS);
  for (int daysRented = 1; daysRented <= points.length; daysRented++) {
   assertEquals(points[daysRented-1], aMovie.getFrequentRenterPoints(daysRented));
  }
 }

 @Test  public void testFrequentRenterPointsNewRelease() {
  int points[] = { 1, 2, 2, 2 };
  Movie aMovie = new Movie(ANY_TITLE,Movie.NEW_RELEASE);
  for (int daysRented = 1; daysRented <= points.length; daysRented++) {
   assertEquals(points[daysRented-1], aMovie.getFrequentRenterPoints(daysRented));
  }
 }

 @Test  public void testChargeForRegular() {
  double charges[] = { 2.0, 2.0, 3.5, 5.0, 6.5, 8.0 };
  Movie aMovie = new Movie(ANY_TITLE,Movie.REGULAR);
  for (int daysRented = 1; daysRented <= charges.length; daysRented++) {
   assertEquals(charges[daysRented-1], aMovie.getCharge(daysRented),0);
  }
 }

 @Test  public void testChargeForChildrens() {
  double charges[] = { 1.5, 1.5, 1.5, 3.0, 4.5, 6.0, 7.5 };
  Movie aMovie = new Movie(ANY_TITLE,Movie.CHILDRENS);
  for (int daysRented = 1; daysRented <= charges.length; daysRented++) {
   assertEquals(charges[daysRented-1], aMovie.getCharge(daysRented),0);
  }
 }

 @Test  public void testChargeNewRelease() {
  double charges[] = { 3.0, 6.0, 9.0, 12.0 };
  Movie aMovie = new Movie(ANY_TITLE,Movie.NEW_RELEASE);
  for (int daysRented = 1; daysRented <= charges.length; daysRented++) {
   assertEquals(charges[daysRented-1], aMovie.getCharge(daysRented),0);
  }
 }
}

Añadimos este test a la suite y volvemos a lanzarlos todos.

En resumen, el cambio que hemos hecho ha consistido en crear los métodos Movie.getCharge(days:int) y Movie.getFrequentRenterPoints(days:int) para alojar ahí la lógica de los métodos homónimos en la clase Rental y que dependían de una propiedad de la clase Movie.

En la siguiente entrega (la última), donde haremos un cambio muy serio al diseño puesto que introduciremos nuevas clases (entre ellas Price), podremos ver cómo recuperaremos la inversión que acabamos de hacer en los nuevos tests.

Hace ya un par de semanas que publiqué la última entrega de “Refactoring en Español”. Voy a seguir con el ejemplo de Fowler en el libro “Refactoring” por el punto donde lo dejamos.

Removing Temps

Antes de comenzar hacemos el siguiente refactor “sintáctico”.

Reordenamos en Customer.statement para poner juntas estas lineas y poder cambiar
lo siguiente:

  Enumeration<Rental> rentals = _rentals.elements();
  while (rentals.hasMoreElements()) {
   Rental each = (Rental) rentals.nextElement();
   ...
  }

por:

  for (Rental each: _rentals ) {
    ...
  }

TESTS. Sí, aunque sea un cambio trivial, refactorizar consiste en hacer pequeños cambios pero siempre muy seguros; por eso lanzamos las pruebas unitarias cada vez que realizamos un cambio en nuestro código.

A partir de aquí podemos ver más claramente la necesidad/posibilidad de
trabajar sobre las variables totalAmount y frequentRenterPoints.

Primero trabajamos con totalAmount y creamos el método getTotalCharge como sigue:

 /**
  * Rolls over the list of rentals calculating
  * the total amount to be charged.
  *
  * @return   */
 private double getTotalCharge() {
  double totalAmount = 0;
  for (Rental each: _rentals ) {
   totalAmount += each.getCharge();
  }
  return totalAmount;
 }

Obsérvese que lo hacemos replicando el bucle que recorre la lista de Rentals y
moviendo el código que suma la cantidad correspondiente del bucle original al
del método (que se ejecuta al final del bucle original y fuera de éste).

Este cambio es conceptualmente difícil de concebir e implementar (este ejemplo es
un ejercicio de laboratorio y en la práctica nos podemos encontrar con casos mucho
más difíciles de ver).

El método statement queda como sigue después de este cambio:

 public String statement() {
  int frequentRenterPoints = 0;
  String result = "Rental Record for " + getName() + "\n";
  for (Rental each: _rentals ) {
   frequentRenterPoints += each.getFrequentRenterPoints();
   // show figures for this rental
   result += "\t" + each.getMovie().getTitle() + "\t"
     + String.valueOf(each.getCharge()) + "\n";
   totalAmount += each.getCharge();
  }
  // add footer lines
  result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n";
  result += "You earned " + String.valueOf(frequentRenterPoints)
    + " frequent renter points";
  return result;
 }

TESTS. En este caso está más justificado que nunca puesto que no se trata de un cambio nada trivial.

A continuación aplicamos la misma receta a frequentRenterPoints

El método statement queda finalmente como sigue:

 public String statement() {
  String result = "Rental Record for " + getName() + "\n";
  for (Rental each: _rentals ) {
   // show figures for this rental
   result += "\t" + each.getMovie().getTitle() + "\t"
     + String.valueOf(each.getCharge()) + "\n";
  }
  // add footer lines
  result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n";
  result += "You earned " + String.valueOf(getTotalFrequentRenterPoints())
    + " frequent renter points";
  return result;
 }

TESTS. No olvidemos ejecutarlos después de cada cambio.

Finalmente, incidir en el comentario que hace Fowler sobre el hecho de que, tras este
cambio, tenemos tres bucles que recorren tres veces la lista de Rentals. Esto lo
tendremos en cuenta cuando estemos mejorando el rendimiento, pero no ahora porque
lo que nos preocupa es mejorar el diseño (sin incumplir con los requisitos), y con
los cambios efectuados hemos mejorado el diseño y la legibilidad del código, lo
que nos llevará a reducir los errores y aumentará la reusabilidad. (El ejemplo de
Fowler es muy bueno: ahora podemos escribir un método htmlStatement que imprime
el recibo en formato HTML, pero reutilizando todo el código que no tiene que ver
con la estética del mismo).