Kent Beck es experto en proponer ideas aparentemente sencillas capaces de generar efectos enormes. Hoy hablaremos del flujo Test and Revert or Commit.
De qué estamos hablando
Test and Commit or Revert es un flujo de trabajo automatizado que consiste en que, tras efectuar un cambio en código de producción, se lanzan los tests. Si estos pasan se hace un commit al repositorio con el cambio. Por contra, si los test fallan se revierte el cambio y volvemos al principio.
Es importante subrayar que este flujo está automatizado con un script que se lanza de forma manual o automáticamente cuando se guardan los cambios en los archivos.
En muchos aspectos, la idea de borrar los cambios que hacen fallar los tests parece un tanto radical. Sin embargo, es una de las claves de este flujo de trabajo. Lo que se intenta favorecer es introducir cambios en el código de producción en forma de pequeños refactors seguros, de tal modo que el código siempre estaría en un estado listo para despliegue. Es la parte de borrado la que estimula que los cambios sean los más pequeños posibles, para evitar perder mucho tiempo o volumen de trabajo.
El ejemplo
Voy a explicar este flujo realizando el ejercicio Tennis Game Kata, un ejercicio que consiste en refactorizar un código encargado de llevar la puntuación de un juego de tenis, con su característica forma de puntuar. La kata contiene un test que verifica todos los resultados. En esta propuesta específica se ofrecen diferentes versiones de TennisGame, con distintos problemas de diseño, pero en este artículo, me centraré en la primera versión TennisGame1, que se muestra a continuación:
public class TennisGame1 implements TennisGame {
private int m_score1 = 0;
private int m_score2 = 0;
private String player1Name;
private String player2Name;
public TennisGame1(String player1Name, String player2Name) {
this.player1Name = player1Name;
this.player2Name = player2Name;
}
public void wonPoint(String playerName) {
if (playerName == "player1")
m_score1 += 1;
else
m_score2 += 1;
}
public String getScore() {
String score = "";
int tempScore=0;
if (m_score1==m_score2)
{
switch (m_score1)
{
case 0:
score = "Love-All";
break;
case 1:
score = "Fifteen-All";
break;
case 2:
score = "Thirty-All";
break;
default:
score = "Deuce";
break;
}
}
else if (m_score1>=4 || m_score2>=4)
{
int minusResult = m_score1-m_score2;
if (minusResult==1) score ="Advantage player1";
else if (minusResult ==-1) score ="Advantage player2";
else if (minusResult>=2) score = "Win for player1";
else score ="Win for player2";
}
else
{
for (int i=1; i<3; i++)
{
if (i==1) tempScore = m_score1;
else { score+="-"; tempScore = m_score2;}
switch(tempScore)
{
case 0:
score+="Love";
break;
case 1:
score+="Fifteen";
break;
case 2:
score+="Thirty";
break;
case 3:
score+="Forty";
break;
}
}
}
return score;
}
}
Cómo es el flujo Test && Commit || Revert
Entonces, ¿cómo funciona Test and Commit or Revert? Es bastante sencillo: se realiza un cambio en el código de producción. Si los tests pasan, el cambio es confirmado haciendo commit al repositorio. En cambio, si los tests dejan de pasar, se elimina el cambio. La clave es que esto se hace de forma automática con un script muy simple que automatiza la secuencia de acciones y las decisiones. Es decir, lanzas un script que ejecuta los tests y realiza el commit si estos pasan correctamente o revierte los cambios en caso contrario.
Por supuesto, esto tiene algunas implicaciones. El objetivo de este flujo es favorecer pequeños cambios inocuos en el proceso de refactor. Si optamos por hacer cambios muy grandes, perderemos mucho trabajo si los tests no pasan, pues los cambios se borrarán. El script nos fuerza a movernos en pasos realmente pequeños, de modo que podamos ser muy conscientes de qué código hemos introducido y cuál ha sido su efecto en el comportamiento de la unidad bajo test. Estos pasos pequeños, en el caso de fallar los tests, suponen solo una pequeña pérdida que puede generar un aprendizaje mucho más importante.
Veamos ahora un ejemplo de ambos flujos, empezando por el positivo:
Partimos de una situación en la que tenemos tests que cubren la pieza de código que queremos refactorizar y aplicamos un cambio. Por ejemplo, este método hace una comparación usando el signo ==.
public void wonPoint(String playerName) {
if (playerName == "player1")
m_score1 += 1;
else
m_score2 += 1;
}
Sería más limpio utilizar equals, así que realizamos ese cambio:
public void wonPoint(String playerName) {
if (playerName.equals("player1"))
m_score1 += 1;
else
m_score2 += 1;
}
Una vez realizado el cambio ejecutamos los tests. Como los tests seguirán pasando, el cambio se consolida.
Intentemos ver ahora qué pasa si el cambio hace que falle algún test. Vamos a forzarlo un poco cambiando alguna de las fórmulas de manera que podamos ver el efecto rápidamente:
if (m_score1==m_score2)
{
switch (m_score1)
{
case 0:
score = "Love-All";
break;
case 1:
score = "Fifteen-All";
break;
case 2:
score = "Thirty-All";
break;
default:
score = "Deuce";
break;
}
}
Por ejemplo, eliminamos el sufijo -All simplemente para ver cómo falla:
if (m_score1==m_score2)
{
switch (m_score1)
{
case 0:
score = "Love";
break;
case 1:
score = "Fifteen";
break;
case 2:
score = "Thirty";
break;
default:
score = "Deuce";
break;
}
}
Con este cambio fallarán varios tests, así que el script lo revertirá automáticamente.
if (m_score1==m_score2)
{
switch (m_score1)
{
case 0:
score = "Love-All";
break;
case 1:
score = "Fifteen-All";
break;
case 2:
score = "Thirty-All";
break;
default:
score = "Deuce";
break;
}
}
Algunas cuestiones a tener en cuenta
Aunque el script original es literalmente test && commit || revert, en la práctica necesitaremos algún retoque. Veamos los más habituales:
- Es posible que necesitemos compilar antes de ejecutar los tests, de tal modo que si la compilación falla no se ejecute el resto del script. Esto nos permite evitar que se destruya el trabajo por tener algún typo o bien por algunos errores que no tienen que ver con la lógica del programa. No se trata de penalizar los errores al escribir, sino los cambios del comportamiento de la unidad bajo test.
- En cuanto a la fase de
revert, nos interesa que únicamente se reviertan los cambios del código de producción. Esto es porque si se revierten también los tests, es imposible introducir tests nuevos y hacer TDD. A esto se le conoce como el problema de los tests que se desvanecen o vanishing tests. Recuerda que los tests de TDD se hacen antes de que exista código de producción que los haga pasar, por tanto, al ejecutarlos no pasarán por definición y el script los borrará. - Usar un watcher, ¿sí o no?: Es frecuente usar un watcher para que el script se ejecute automáticamente cuando se guardan los cambios en el fichero. No tengo una opinión muy consolidada sobre esto. Veo el inconveniente de que tal vez pierdas trabajo parcial si tu IDE guarda el archivo periódicamente y puede ser frustrante. Por otro lado, puede ayudarte a tener más cuidado y añadir nada más que cambios realmente pequeños.
¿Es compatible TCR con TDD?
Pues sí, siempre y cuando tengas presente aplicar el revert únicamente al código de producción. De este modo, los tests que vayas escribiendo no se borrarán automáticamente, mientras que los cambios en el código de producción que hagan fallar algo serán eliminados.
De hecho, TCR complementa TDD incidiendo en que los cambios durante la fase de refactor sean pequeños e inocuos.
Preparar el proyecto para TCR
Puedes usar este repo de Isidro López para obtener scripts en diversos lenguajes. De hecho, he usado el de java para este ejemplo y ha bastado copiarlo en el repositorio para poder trabajar. Los scripts contemplan las observaciones que he comentado antes.
Aunque está implícito es su definición, el proyecto tiene que estar bajo control de versiones. Por tanto, en algún momento tendrás que hacer un commit inicial para poner todos los archivos implicados bajo seguimiento. De otro modo, el flujo TCR no los tendrá en cuenta. Por otro lado, los commits generados automáticamente serán muy pequeños y, en algunos casos, poco significativos, por lo que antes de mezclar los cambios deberías agruparlos haciendo un squash para agruparlos en una unidad más significativa.
Aplicar cambios grandes con TCR
En el ejemplo anterior he mostrado cambios muy pequeños y sencillos. Sin embargo, en el mundo real nos encontraremos con la necesidad de realizar refactor de mucho más calado. TCR nos puede ayudar a introducirlo de manera indolora para el código de producción, de modo que podamos tener el proyecto listo para desplegar en cualquier momento con seguridad.
Lo interesante aquí es que TCR nos permite experimentar. Si el camino que seguimos nos lleva a una solución errónea, los cambios se borrarán en cuanto fallen los tests.
Emprendiendo el camino equivocado
En este ejercicio hay varias oportunidades de refactor. Una de las más obvias podría ser la introducción de un concepto Score que podría ser representado mediante un Enum.
enum Score {
LOVE, FIFTEEN, THIRTY, FORTY
}
Introducir el enum dentro de la clase TennisGame1 no genera ningún problema para pasar los tests, por lo que al ejecutarse el flujo TCR se conserva. Lo interesante ahora es introducir el concepto en el código sin romper los tests. Así por ejemplo, podríamos usar una estrategia de cambio paralelo. Introducimos código que usa el enumerable mientras mantenemos el anterior. En un primer momento vamos a añadir código que no será usado, hasta que estamos en condiciones de reemplazar el original.
El siguiente cambio, sería gestionar lo que le ocurre a Score cada vez que se gana un punto:
enum Score {
LOVE, FIFTEEN, THIRTY, FORTY;
public Optional<Score> wonPoint() {
switch (this) {
case LOVE: return Optional.of(FIFTEEN);
case FIFTEEN: return Optional.of(THIRTY);
case THIRTY: return Optional.of(FORTY);
default: return Optional.empty();
}
}
}
De nuevo, este cambio introduce código que, al no usarse, no produce efectos en los tests, por lo que es aceptado por TCR. Sin embargo, nuestro siguiente paso dará problemas:
public void wonPoint(String playerName) {
if (playerName.equals("player1")) {
m_score1 += 1;
score1 = score1.wonPoint().get();
}
else {
m_score2 += 1;
score2 = score2.wonPoint().get();
}
}
Los tests no pasan, por lo que el cambio es descartado. Esto ocurre porque en algún momento, Score.wonPoint().get() nos dará valores vacíos y no hemos previsto gestionar esa situación.
En realidad, cuando la puntuación está en FORTY y se gana el punto, sabemos que pueden pasar tres cosas: se gana el juego si la puntuación de la otra jugadora es menor que FORTY (GAME), pero se obtiene ADVANTAGE si estaban empatadas a FORTY, mientras que se ponen en DEUCE si la otra jugadora estaba en FORTY. Es decir, a partir de cierta puntuación los cambios dependen de la puntuación de la otra jugadora.
Por lo tanto, es posible que este enfoque esté equivocado, Estamos empezando a añadir demasiado código, seguramente habrá una forma más sencilla de proceder. Puede que para llegar al mismo lugar, pero por un camino mucho más seguro.
El problema es que estamos intentando rediseñar la solución antes de saber lo suficiente y de haber organizado el código de una forma capaz de revelar los conceptos subyacentes. Obviamente añadir código no usado permitirá que los tests sigan pasando, pero en el momento de integrarlo, es muy posible que tengamos dificultades y, de todos modos, los cambios no nos proporcionan ninguna información.
En consecuencia, vamos a usar otro enfoque, centrándonos en resolver los smells más visibles.
Pequeños cambios que conducen a un rediseño
Vamos a volver al punto de partida reseteando la rama de trabajo. Lo que vamos a buscar son oportunidades de refactor más sencillas, que vayan añadiendo claridad al código eliminando los smells más evidentes.
Así, por ejemplo, podríamos trabajar con las condicionales. Tenemos varias posibilidades:
- Aplicar
early return, para eliminarelseyelse if. - Extraer las patas de las condicionales a sus propios métodos.
El cuerpo de getScore tiene tres posibles rutas. Centrémonos en la primera.
Claramente, la primera gestiona los casos de empate y se puede ver que el resultado obtenido aquí se podría devolver desde la misma rama, ya que se asigna el valor a la variable score que es la que se retorna.
if (m_score1==m_score2)
{
switch (m_score1)
{
case 0:
score = "Love-All";
break;
case 1:
score = "Fifteen-All";
break;
case 2:
score = "Thirty-All";
break;
default:
score = "Deuce";
break;
}
}
Para probar esto, simplemente voy a devolver score tras el switch:
if (m_score1==m_score2)
{
switch (m_score1)
{
case 0:
score = "Love-All";
break;
case 1:
score = "Fifteen-All";
break;
case 2:
score = "Thirty-All";
break;
default:
score = "Deuce";
break;
}
return score;
}
Y lanzo el script TCR, con el resultado de que los tests siguen pasando y el cambio se consolida en un commit.
Esto debería permitirme extraer la rama a un método. Es un refactor automático y los tests deberían seguir pasando.
if (m_score1==m_score2)
{
return tieScore();
}
private String tieScore() {
String score;
switch (m_score1)
{
case 0:
score = "Love-All";
break;
case 1:
score = "Fifteen-All";
break;
case 2:
score = "Thirty-All";
break;
default:
score = "Deuce";
break;
}
return score;
}
Efectivamente, los tests pasan y el cambio se consolida. Además, podemos hacer un poco más sencilla la estructura eliminando la variable temporal y los breaks:
private String tieScore() {
String score;
switch (m_score1) {
case 0:
return "Love-All";
case 1:
return "Fifteen-All";
case 2:
return "Thirty-All";
default:
return "Deuce";
}
}
De nuevo, este cambio es consolidado por el flujo TCR y nos deja ver posibles evoluciones de este método, pero vamos a ir primero a trabajar con la segunda rama de la condicional.
else if (m_score1 >= 4 || m_score2 >= 4) {
int minusResult = m_score1 - m_score2;
if (minusResult == 1) score = "Advantage player1";
else if (minusResult == -1) score = "Advantage player2";
else if (minusResult >= 2) score = "Win for player1";
else score = "Win for player2";
}
De nuevo, es posible retornar el valor de score desde la misma rama, así que vamos a probarlo con este simple cambio:
else if (m_score1 >= 4 || m_score2 >= 4) {
int minusResult = m_score1 - m_score2;
if (minusResult == 1) score = "Advantage player1";
else if (minusResult == -1) score = "Advantage player2";
else if (minusResult >= 2) score = "Win for player1";
else score = "Win for player2";
return score;
}
El cambio se consolida pues los tests pasan igualmente. Podemos extraer la rama a un método, como hicimos antes, de modo que podamos trabajar aisladamente con él. En este apartado se tratan dos situaciones (obtener ventaja o ganar). De momento, voy a mantenerlas unidas bajo el nombre finishGameScore y me volveré a ocupar de ello más adelante:
else if (m_score1 >= 4 || m_score2 >= 4) {
return finishGameScore();
}
private String finishGameScore() {
String score;
int minusResult = m_score1 - m_score2;
if (minusResult == 1) score = "Advantage player1";
else if (minusResult == -1) score = "Advantage player2";
else if (minusResult >= 2) score = "Win for player1";
else score = "Win for player2";
return score;
}
El cambio se consolida, señal de que vamos bien. Este método está bastante liado con tanto if/else, pero creo que prefiero seguir la ruta inicial y gestionar la última rama de la condicional.
else {
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
switch (tempScore) {
case 0:
score += "Love";
break;
case 1:
score += "Fifteen";
break;
case 2:
score += "Thirty";
break;
case 3:
score += "Forty";
break;
}
}
Esta rama es bastante fea y tiene dos dependencias, ya que usa dos variables que se inicializan en el cuerpo principal. Debería moverlas dentro de la rama. Pero antes, quiero devolver score desde la propia rama, para ver si puedo eliminar el return score del flujo principal.
else {
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
switch (tempScore) {
case 0:
score += "Love";
break;
case 1:
score += "Fifteen";
break;
case 2:
score += "Thirty";
break;
case 3:
score += "Forty";
break;
}
}
return score;
}
De hecho, el compilador me obliga a quitar el return score porque las condicionales cubren todos los posibles paths de ejecución. Al pasar el flujo TCR se convalida este cambio y puedo pensar en mover la inicialización de las variables necesarias dentro de la propia rama, que es la única interesada en ellas:
else {
String score = "";
int tempScore = 0;
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
switch (tempScore) {
case 0:
score += "Love";
break;
case 1:
score += "Fifteen";
break;
case 2:
score += "Thirty";
break;
case 3:
score += "Forty";
break;
}
}
return score;
}
El flujo TCR aprueba este cambio, por lo que ya puedo extraer este bloque, que aún no entiendo, a un método privado, dejando getScore muy sencillito:
public String getScore() {
if (m_score1 == m_score2) {
return tieScore();
} else if (m_score1 >= 4 || m_score2 >= 4) {
return finishGameScore();
} else {
return normalScore();
}
}
private String normalScore() {
String score = "";
int tempScore = 0;
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
switch (tempScore) {
case 0:
score += "Love";
break;
case 1:
score += "Fifteen";
break;
case 2:
score += "Thirty";
break;
case 3:
score += "Forty";
break;
}
}
return score;
}
Estos cambios también son consolidados por el flujo TCR. En este momento, voy a intentar reflexionar acerca de lo que me dice el código sobre el dominio.
Por ejemplo, parece claro que no necesitaba el objeto Score que intenté definir al principio. Es un caso de una generalización prematura, al intentar introducir un concepto que tal vez no sea necesario y para el que no tenemos todavía información suficiente.
La línea de refactoring seguida, en cambio, nos ha ayudado a identificar tres situaciones o tipos de resultado: empate (tie), finalización del juego y puntuación normal. De hecho, la finalización del juego esconde dos circunstancias diferentes: ganar el juego y obtener ventaja. Este podría ser un buen hilo del que seguir tirando y separar ambas circunstancias. Antes de eso, sin embargo podríamos eliminar los else en getScore.
public String getScore() {
if (m_score1 == m_score2) {
return tieScore();
}
if (m_score1 >= 4 || m_score2 >= 4) {
return finishGameScore();
}
return normalScore();
}
Este cambio es validado por TCR. Su objetivo es hacer más fácil romper las dos acciones contenidas en finishGameScore.
Primero intentaré separarlas en el propio método. De nuevo, podríamos plantear quitar los else mediante early return.
private String finishGameScore() {
int minusResult = m_score1 - m_score2;
if (minusResult == 1) return "Advantage player1";
else if (minusResult == -1) return "Advantage player2";
else if (minusResult >= 2) return "Win for player1";
else return "Win for player2";
}
El cambio es conservado por TCR, así que ahora voy a intentar separar ambas acciones. Y ahora sí quitamos los else:
private String finishGameScore() {
int minusResult = m_score1 - m_score2;
if (minusResult == 1) return "Advantage player1";
if (minusResult == -1) return "Advantage player2";
if (minusResult >= 2) return "Win for player1";
return "Win for player2";
}
Otro cambio que se consolida.
La separación de las acciones de ventaja y ganar el juego parece clara, pero vamos a hacer primero un pequeño cambio que hará que sea más sencillo:
private String finishGameScore() {
int minusResult = m_score1 - m_score2;
if (minusResult == 1) return "Advantage player1";
if (minusResult == -1) return "Advantage player2";
int winResult = m_score1 - m_score2;
if (winResult >= 2) return "Win for player1";
return "Win for player2";
}
Con este cambio, quedan claras cuáles son las dos acciones. ¿Cuál es la condición para identificar una u otra? La diferencia de puntos: si es 1 ó -1, se trata de ventaja, y si la diferencia es de dos o más puntos, es que uno de los jugadores ha ganado el juego. Podríamos expresarlo así:
private String finishGameScore() {
int minusResult = m_score1 - m_score2;
if (Math.abs(minusResult) == 1) {
if (minusResult == 1) return "Advantage player1";
if (minusResult == -1) return "Advantage player2";
}
int winResult = m_score1 - m_score2;
if (winResult >= 2) return "Win for player1";
return "Win for player2";
}
El cambio resulta consolidado, así que aplicamos la misma idea en el otro bloque:
private String finishGameScore() {
int minusResult = m_score1 - m_score2;
if (Math.abs(minusResult) == 1) {
if (minusResult == 1) return "Advantage player1";
if (minusResult == -1) return "Advantage player2";
}
int winResult = m_score1 - m_score2;
if (Math.abs(winResult) >= 2) {
if (winResult >= 2) return "Win for player1";
return "Win for player2";
}
return "";
}
El cambio ha sobrevivido al flujo TCR, aunque el resultado no ha quedado muy bien al tener que retornar una cadena vacía porque no se contempla el caso de que haya empate (no se puede producir en este método). Para continuar, vamos a deshacernos de las variables temporales:
private String finishGameScore() {
if (Math.abs(m_score1 - m_score2) == 1) {
if (m_score1 - m_score2 == 1) return "Advantage player1";
if (m_score1 - m_score2 == -1) return "Advantage player2";
}
int winResult = m_score1 - m_score2;
if (Math.abs(winResult) >= 2) {
if (winResult >= 2) return "Win for player1";
return "Win for player2";
}
return "";
}
Este cambio también resulta consolidado. Hacemos lo mismo con la otra variable.
private String finishGameScore() {
if (Math.abs(m_score1 - m_score2) == 1) {
if (m_score1 - m_score2 == 1) return "Advantage player1";
if (m_score1 - m_score2 == -1) return "Advantage player2";
}
if (Math.abs(m_score1 - m_score2) >= 2) {
if (m_score1 - m_score2 >= 2) return "Win for player1";
return "Win for player2";
}
return "";
}
Y ahora vamos a extraer los métodos que generan el marcador para el caso de ventaja y para el caso de haber ganado el juego:
private String finishGameScore() {
if (Math.abs(m_score1 - m_score2) == 1) {
return advantageScore();
}
if (Math.abs(m_score1 - m_score2) >= 2) {
if (m_score1 - m_score2 >= 2) return "Win for player1";
return "Win for player2";
}
return "";
}
private String advantageScore() {
if (m_score1 - m_score2 == 1) return "Advantage player1";
return "Advantage player2";
}
Este refactor permite que pasen los tests. Todavía podemos reducir un poco de código, ya que tiene sentido utilizar un ternario:
private String advantageScore() {
return m_score1 - m_score2 == 1 ? "Advantage player1" : "Advantage player2";
}
De nuevo, el refactor pasa el filtro del TCR y extraemos el método winningScore en un solo paso en la confianza de que no se borrará.
private String finishGameScore() {
if (Math.abs(m_score1 - m_score2) == 1) {
return advantageScore();
}
if (Math.abs(m_score1 - m_score2) >= 2) {
return winningScore();
}
return "";
}
private String winningScore() {
return m_score1 - m_score2 >= 2 ? "Win for player1" : "Win for player2";
}
El siguiente paso será deshacernos del método finishGameScore. Para esto primero haré un inline del método:
public String getScore() {
if (m_score1 == m_score2) {
return tieScore();
}
if (m_score1 >= 4 || m_score2 >= 4) {
if (Math.abs(m_score1 - m_score2) == 1) {
return advantageScore();
}
if (Math.abs(m_score1 - m_score2) >= 2) {
return winningScore();
}
return "";
}
return normalScore();
}
No ha quedado muy bonito, pero los tests pasan y el cambio se mantiene. Ahora podemos limpiarlo tranquilamente. El primer es quitar la línea return "" .
public String getScore() {
if (m_score1 == m_score2) {
return tieScore();
}
if (m_score1 >= 4 || m_score2 >= 4) {
if (Math.abs(m_score1 - m_score2) == 1) {
return advantageScore();
}
if (Math.abs(m_score1 - m_score2) >= 2) {
return winningScore();
}
}
return normalScore();
}
Ahora fusionamos las condiciones para tener un único nivel de indentación y hacer similares todas las líneas:
public String getScore() {
if (m_score1 == m_score2) {
return tieScore();
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) == 1) {
return advantageScore();
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) >= 2) {
return winningScore();
}
return normalScore();
}
Este cambio parece muy grande, aunque no lo es tanto y los tests siguen pasando por lo que validamos el cambio. Ahora tenemos todos los tipos de marcador y las condiciones a las que obedecen. Esto sugiere fuertemente la posibilidad de introducir objetos que encapsulen estas reglas de negocio.
Este refactor es considerable, pero básicamente consiste en crear una clase anidada TieScore y mover a ella el método correspondiente:
private String tieScore() {
return new TieScore(m_score1).getScore();
}
class TieScore
{
private final int m_score1;
TieScore(int m_score1) {
this.m_score1 = m_score1;
}
public String getScore() {
switch (m_score1) {
case 0:
return "Love-All";
case 1:
return "Fifteen-All";
case 2:
return "Thirty-All";
default:
return "Deuce";
}
}
}
Este cambio ha sido grande, pero los tests pasan sin inmutarse y se puede incorporar sin problemas.
A continuación, se trataría de hacer lo mismo con todos los demás métodos. El resultado final tras proceder paso a paso y ejecutar el flujo TCR cada vez sería más o menos así:
private String normalScore() {
return new NormalScore(m_score1, m_score2).getScore();
}
private String winningScore() {
return new WinningScore(m_score1, m_score2).getScore();
}
private String advantageScore() {
return new AdvantageScore(m_score1, m_score2).getScore();
}
private String tieScore() {
return new TieScore(m_score1).getScore();
}
class TieScore
{
private final int m_score1;
TieScore(int m_score1) {
this.m_score1 = m_score1;
}
public String getScore() {
switch (m_score1) {
case 0:
return "Love-All";
case 1:
return "Fifteen-All";
case 2:
return "Thirty-All";
default:
return "Deuce";
}
}
}
class WinningScore
{
private int m_score1;
private int m_score2;
public WinningScore(int m_score1, int m_score2) {
this.m_score1 = m_score1;
this.m_score2 = m_score2;
}
public String getScore()
{
return m_score1 - m_score2 >= 2 ? "Win for player1" : "Win for player2";
}
}
class AdvantageScore
{
private int m_score1;
private int m_score2;
public AdvantageScore(int m_score1, int m_score2) {
this.m_score1 = m_score1;
this.m_score2 = m_score2;
}
public String getScore()
{
return m_score1 - m_score2 == 1 ? "Advantage player1" : "Advantage player2";
}
}
class NormalScore
{
private int m_score1;
private int m_score2;
public NormalScore(int m_score1, int m_score2) {
this.m_score1 = m_score1;
this.m_score2 = m_score2;
}
private String getScore() {
String score = "";
int tempScore = 0;
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
switch (tempScore) {
case 0:
score += "Love";
break;
case 1:
score += "Fifteen";
break;
case 2:
score += "Thirty";
break;
case 3:
score += "Forty";
break;
}
}
return score;
}
}
En este punto, podemos hacer inline de los métodos llamados desde getScore, lo que daría el siguiente resultado:
public String getScore() {
if (m_score1 == m_score2) {
return new TieScore(m_score1).getScore();
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) == 1) {
return new AdvantageScore(m_score1, m_score2).getScore();
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) >= 2) {
return new WinningScore(m_score1, m_score2).getScore();
}
return new NormalScore(m_score1, m_score2).getScore();
}
Aquí se pueden observar varias cosas:
- Las complejas expresiones condicionales que definen las reglas de aplicación de cada tipo de
Score. - Todos los objetos
Scorecomparten una interfaz, la cual podríamos extraer. Ya, de paso, el métodogetScorede los objetos*Scorepodría renombrarse ascore. - Las reglas definen qué tipo concreto de objeto
Scorese debe usar para preguntarle por el resultado actual del partido. Tendría sentido trasladar la responsabilidad de definir qué objetoScorenos proporcionará el resultado.
Vamos por partes. Renombrar el método no debería causar problemas:
public String getScore() {
if (m_score1 == m_score2) {
return new TieScore(m_score1).score();
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) == 1) {
return new AdvantageScore(m_score1, m_score2).score();
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) >= 2) {
return new WinningScore(m_score1, m_score2).score();
}
return new NormalScore(m_score1, m_score2).score();
}
Y, por supuesto, no lo hace. He ido aplicando el flujo TCR en cada cambio. Introducir la interfaz, tampoco debería generar ningún problema y nos facilitará las cosas en un momento:
public interface GameScore {
String score();
}
El siguiente paso será declarar todos los objetos *Score como implementadores de GameScore:
class WinningScore implements GameScore
{
private int m_score1;
private int m_score2;
public WinningScore(int m_score1, int m_score2) {
this.m_score1 = m_score1;
this.m_score2 = m_score2;
}
@Override
public String score()
{
return m_score1 - m_score2 >= 2 ? "Win for player1" : "Win for player2";
}
}
Esto nos permite separar la obtención del objeto GameScore de la del marcador, extrayendo un método:
public String getScore() {
return getGameScore().score();
}
private GameScore getGameScore() {
if (m_score1 == m_score2) {
return new TieScore(m_score1);
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) == 1) {
return new AdvantageScore(m_score1, m_score2);
}
if ((m_score1 >= 4 || m_score2 >= 4) && Math.abs(m_score1 - m_score2) >= 2) {
return new WinningScore(m_score1, m_score2);
}
return new NormalScore(m_score1, m_score2);
}
Por supuesto, sigo ejecutando el flujo TCR tras los cambios y estos han sido consolidados.
Ahora que hemos separado las responsabilidades es más fácil ver que podríamos trasladarlas a otro objeto. En el tenis, el árbitro es el responsable de llevar el control del marcador, por lo que tendría sentido reflejar esto en el código:
public String getScore() {
return getGameScore().score();
}
private GameScore getGameScore() {
return new Umpire(m_score1, m_score2).getGameScore();
}
class Umpire {
private int score1;
private int score2;
public Umpire(int score1, int score2) {
this.score1 = score1;
this.score2 = score2;
}
public GameScore getGameScore() {
if (score1 == score2) {
return new TieScore(score1);
}
if ((score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) == 1) {
return new AdvantageScore(score1, score2);
}
if ((score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) >= 2) {
return new WinningScore(score1, score2);
}
return new NormalScore(score1, score2);
}
}
Estos cambios implican bastantes líneas, pero en el fondo son bastante sencillos y seguros. De hecho, el TCR los consolida. Gracias a estos cambios, TennisGame es ahora una clase muy pequeña, que delega tareas en otras clases especializadas.
Por supuesto, podemos seguir con el refactor. En la clase Umpire tenemos una serie de reglas expresadas mediante condicionales. Hagámoslas más explícitas, empezando por el empate a puntos:
public GameScore getGameScore() {
if (isTie()) {
return new TieScore(score1);
}
if ((score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) == 1) {
return new AdvantageScore(score1, score2);
}
if ((score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) >= 2) {
return new WinningScore(score1, score2);
}
return new NormalScore(score1, score2);
}
private boolean isTie() {
return score1 == score2;
}
Este cambio también se consolida y podemos aplicarlo a las otras reglas. El método getGameScore quedaría así:
public GameScore getGameScore() {
if (isTie()) {
return new TieScore(score1);
}
if (isAdvantage()) {
return new AdvantageScore(score1, score2);
}
if (isWin()) {
return new WinningScore(score1, score2);
}
return new NormalScore(score1, score2);
}
private boolean isWin() {
return (score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) >= 2;
}
private boolean isAdvantage() {
return (score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) == 1;
}
private boolean isTie() {
return score1 == score2;
}
En este punto, parece que podríamos extraer objetos para encapsular estas reglas. Es posible que estemos empezando a vislumbrar algún patrón, pero ciertamente tenemos que seguir trabajando en pasos pequeños. Recuerda, como vimos al principio, que intentar adelantarse a lo que nos va indicando el código puede llevarnos a problemas.
private boolean isWin() {
return new WinRule().applies(score1, score2);
}
// ...
class WinRule {
public boolean applies(int score1, int score2) {
return (score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) >= 2;
}
}
Aquí tenemos un ejemplo. Los tests pasan y el cambio es consolidado, por lo que hacemos lo mismo con las otras reglas, procediendo paso a paso:
private boolean isWin() {
return new WinRule().applies(score1, score2);
}
private boolean isAdvantage() {
return new AdvantageRule().applies(score1, score2);
}
private boolean isTie() {
return new TieRule().applies(score1, score2);
}
// ...
class WinRule {
public boolean applies(int score1, int score2) {
return (score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) >= 2;
}
}
class AdvantageRule {
private boolean applies(int score1, int score2) {
return (score1 >= 4 || score2 >= 4) && Math.abs(score1 - score2) == 1;
}
}
class TieRule {
private boolean applies(int score1, int score2) {
return score1 == score2;
}
}
Una vez consolidados estos cambios, lo siguiente es hacer inline de los métodos de Umpire, ya que no nos hacen falta. El resultado, después de proceder uno por uno, es:
class Umpire {
private final int score1;
private final int score2;
public Umpire(int score1, int score2) {
this.score1 = score1;
this.score2 = score2;
}
public GameScore getGameScore() {
if (new TieRule().applies(score1, score2)) {
return new TieScore(score1);
}
if (new AdvantageRule().applies(score1, score2)) {
return new AdvantageScore(score1, score2);
}
if (new WinRule().applies(score1, score2)) {
return new WinningScore(score1, score2);
}
return new NormalScore(score1, score2);
}
}
Parece que estemos viendo una cierta estructura aquí, ¿es posible? Tenemos una regla que si se aplica proporciona el GameScore adecuado. Sin embargo, para NormalScore no hay ninguna regla específica. La opción sería introducir una:
class DefaultRule {
public boolean applies(int score1, int score2) {
return true;
}
}
De este modo, el método getGameScore quedaría así:
public GameScore getGameScore() {
if (new TieRule().applies(score1, score2)) {
return new TieScore(score1);
}
if (new AdvantageRule().applies(score1, score2)) {
return new AdvantageScore(score1, score2);
}
if (new WinRule().applies(score1, score2)) {
return new WinningScore(score1, score2);
}
if (new DefaultRule().applies(score1, score2)) {
return new NormalScore(score1, score2);
}
return null;
}
El return null del final no queda muy bien, pero es algo temporal y los tests pasan, por lo que se ejecuta el commit con los cambios.
De momento, la regla DefaultRule, ha quedado de esta manera, lo que nos obliga a usarla siempre como último recurso.
class DefaultRule {
public boolean applies(int score1, int score2) {
return true;
}
}
Esto empieza a parecerse mucho a un patrón de cadena de responsabilidad, pero necesitaríamos hacer algunos cambios. Por ejemplo, los objetos Rule deberían implementar una misma interfaz y ser capaces de proporcionarnos el objeto GameScore adecuado. También necesitamos que cada regla pueda delegar en otra si no se cumple. DefaultRule no delegaría en ninguna otra, garantizando una respuesta por defecto. Esto requiere varios cambios, que vamos a ir haciendo poco a poco. Aquí pondré el ejemplo de una de las reglas:
TieRule tieRule = new TieRule();
if (tieRule.applies(score1, score2)) {
return tieRule.gameScore(score1, score2);
}
class TieRule {
private boolean applies(int score1, int score2) {
return score1 == score2;
}
public GameScore gameScore(int score1, int score2) {
return new TieScore(score1);
}
}
Una vez que hemos aplicado el mismo tratamiento a las cuatro reglas vamos a hacer que implementen la interfaz Rule, extrayéndola primero, lo cual no debería darnos ningún problema para que los cambios se envíen al repositorio:
public interface Rule {
boolean applies(int score1, int score2);
GameScore gameScore(int score1, int score2);
}
Para que funcione la cadena de responsabilidad necesitamos poder enlazar las reglas, de modo que cada una pueda delegar en otra. Lo mejor es empezar dándole soporte a añadir una regla para delegar opcional. Yo prefiero hacerlo en el constructor:
class TieRule implements Rule {
private Rule delegate;
public TieRule(Rule delegate) {
this.delegate = delegate;
}
public TieRule() {}
@Override
public boolean applies(int score1, int score2) {
return score1 == score2;
}
@Override
public GameScore gameScore(int score1, int score2) {
return new TieScore(score1);
}
}
Este cambio permite que los tests sigan pasando por lo que se acepta. Hacemos lo mismo para las demás reglas.
Nuestro siguiente objetivo será gestionar la delegación. Nosotros lo haremos así:
- Si la regla no aplica, delegamos en la siguiente regla si existe.
- Si la regla aplica, devolvemos el
gameScorecorrespondiente. - Hay que tener en cuenta que
DefaultRulesiempre se podrá aplicar, por lo que poniéndola como última de la cadena nos garantiza que tendremos unGameScoreen cualquier caso.
Para implementar esto tenemos que hacerlo en pequeños pasos que no afecten a los tests. Es decir, tenemos varias formas posibles de aproximarnos, pero algunas de ellas mantendrán los test sin pasar durante mucho tiempo. Y como estamos usando TCR, los cambios que hagamos se eliminarán.
Ahora mismo, las reglas únicamente se ejecutarán si aplican, al estar dentro de una condicional que chequea si pueden aplicarse. Eso nos proporciona una red de seguridad para introducir la condición dentro de la propia regla:
class TieRule implements Rule {
private Rule delegate;
public TieRule(Rule delegate) {
this.delegate = delegate;
}
public TieRule() {
}
@Override
public boolean applies(int score1, int score2) {
return score1 == score2;
}
@Override
public GameScore gameScore(int score1, int score2) {
if (applies(score1, score2)) {
return new TieScore(score1);
}
return null;
}
}
Este cambio es aceptado en el flujo TCR. De nuevo, tener que devolver null lo asumimos como algo temporal. Hacemos lo mismo en las demás reglas.
Con esto tenemos la gestión del código que se debe ejecutar si la regla aplica. Ahora nos tocaría gestionar la delegación. Hemos dicho que si la regla no aplica delegamos si existe una regla a la que delegar.
class TieRule implements Rule {
private Rule delegate;
public TieRule(Rule delegate) {
this.delegate = delegate;
}
public TieRule() {
}
@Override
public boolean applies(int score1, int score2) {
return score1 == score2;
}
@Override
public GameScore gameScore(int score1, int score2) {
if (applies(score1, score2)) {
return new TieScore(score1);
}
return delegate.gameScore(score1, score2);
}
}
Los tests no fallan porque, por el momento, esta línea de delegación no se llega a ejecutar. Esto nos permite prepararnos para las siguientes etapas. Aplicamos el mismo cambio en todas las demás Rules.
A continuación lo que nos queda es montar la cadena de responsabilidad. Para ello, primero juntamos todas las líneas que instancian las reglas, otro refactor inocuo:
public GameScore getGameScore() {
TieRule tieRule = new TieRule();
AdvantageRule advantageRule = new AdvantageRule();
WinRule winRule = new WinRule();
DefaultRule defaultRule = new DefaultRule();
if (tieRule.applies(score1, score2)) {
return tieRule.gameScore(score1, score2);
}
if (advantageRule.applies(score1, score2)) {
return advantageRule.gameScore(score1, score2);
}
if (winRule.applies(score1, score2)) {
return winRule.gameScore(score1, score2);
}
if (defaultRule.applies(score1, score2)) {
return defaultRule.gameScore(score1, score2);
}
return null;
}
Y montamos la cadena, teniendo en cuenta que DefaultRule tendría que ser la última. Podríamos hacer que DefaultRule tuviera unas condiciones tales que nos permita ponerla en cualquier posición, pero de momento vamos a dejarla así.
public GameScore getGameScore() {
DefaultRule defaultRule = new DefaultRule();
TieRule tieRule = new TieRule(defaultRule);
AdvantageRule advantageRule = new AdvantageRule(tieRule);
WinRule winRule = new WinRule(advantageRule);
if (tieRule.applies(score1, score2)) {
return tieRule.gameScore(score1, score2);
}
if (advantageRule.applies(score1, score2)) {
return advantageRule.gameScore(score1, score2);
}
if (winRule.applies(score1, score2)) {
return winRule.gameScore(score1, score2);
}
if (defaultRule.applies(score1, score2)) {
return defaultRule.gameScore(score1, score2);
}
return null;
}
Este cambio también se acepta. La cadena queda definida en orden inverso. La primera regla en aplicarse sería WinRule, que delega en AdvantageRule, esta en TieRule y, finalmente, DefaultRule. Por tanto, nos bastaría con ejecutar WinRule y dejar que la delegación haga su magia. ¿Funcionará?:
public GameScore getGameScore() {
DefaultRule defaultRule = new DefaultRule();
TieRule tieRule = new TieRule(defaultRule);
AdvantageRule advantageRule = new AdvantageRule(tieRule);
WinRule winRule = new WinRule(advantageRule);
return winRule.gameScore(score1, score2);
}
Todos los tests pasan, hemos reemplazado cuatro condicionales por una cadena de responsabilidad. Eliminemos variables temporales:
public GameScore getGameScore() {
Rule rules = new WinRule(new AdvantageRule(new TieRule(new DefaultRule())));
return rules.gameScore(score1, score2);
}
De hecho, no necesitamos inicializar la cadena en cada llamada a getGameScore. Podemos promover rules a propiedad e inicializarlo en construcción lo que tiene incluso más sentido desde el punto de vista de dominio, ya que nos dice que el árbitro aplica unas reglas y siempre las mismas:
class Umpire {
private final int score1;
private final int score2;
private final Rule rules = new WinRule(new AdvantageRule(new TieRule(new DefaultRule())));
public Umpire(int score1, int score2) {
this.score1 = score1;
this.score2 = score2;
}
public GameScore getGameScore() {
return rules.gameScore(score1, score2);
}
}
En este punto, convendría volver a examinar el código desde un punto de vista más general, para ver en dónde estamos y qué cambios nos convendría introducir. Por ejemplo, en Umpire estamos pasando datos en construcción que podríamos pasar a getGameScore, pero tenemos que “subir” por el código hasta el punto en el que se empiezan a pasar estos datos. Además son posibles pequeños refactors en nombres, visibilidad de métodos, etc., que podríamos hacer en aplicación de la Camp Rule. Igualmente, algunos constructores de las Rules no se utilizan y podríamos eliminarlos.
Vamos por partes. En este método, podríamos pasar las puntuaciones a getGameScore en lugar de pasarlos para construir Umpire.
private GameScore getGameScore() {
return new Umpire(m_score1, m_score2).getGameScore();
}
Así que hacemos el cambio:
private GameScore getGameScore() {
return new Umpire().getGameScore(m_score1, m_score2);
}
class Umpire {
private final Rule rules = new WinRule(new AdvantageRule(new TieRule(new DefaultRule())));
public GameScore getGameScore(int score1, int score2) {
return rules.gameScore(score1, score2);
}
}
Nos llevamos Umpire como colaborador:
public class TennisGame1 implements TennisGame {
private final Umpire umpire = new Umpire();
// ...
private GameScore getGameScore() {
return umpire.getGameScore(m_score1, m_score2);
}
Y podemos hacer algunas modificaciones en el método getScore, extrayendo una variable y haciendo inline del método privado:
public String getScore() {
GameScore gameScore = umpire.getGameScore(m_score1, m_score2);
return gameScore.score();
}
La clase DefaultRule nunca delega, por lo que no necesita que le pasemos una nueva regla para delegar. Por tanto, podemos eliminar parte de ese código. Así es como está ahora:
class DefaultRule implements Rule {
private Rule delegate;
public DefaultRule(Rule delegate) {
this.delegate = delegate;
}
public DefaultRule() {
}
@Override
public boolean applies(int score1, int score2) {
return true;
}
@Override
public GameScore gameScore(int score1, int score2) {
if (applies(score1, score2)) {
return new NormalScore(score1, score2);
}
return delegate.gameScore(score1, score2);
}
}
Y así es como queda tras el refactor:
class DefaultRule implements Rule {
private Rule delegate;
public DefaultRule() {
}
@Override
public boolean applies(int score1, int score2) {
return true;
}
@Override
public GameScore gameScore(int score1, int score2) {
return new NormalScore(score1, score2);
}
}
Los cambios son consolidados, por lo que podemos seguir adelante. Una alternativa sería hacer que applies introdujese una condición tal que cubriese todos los casos que no cumplen las demás reglas. Eso nos permitiría poner las reglas en cualquier orden, pero también nos obliga a tener en consideración que todas las reglas tendrían que contemplar la posibilidad de no poder delegar.
El resto de Rules no usan el constructor vacío, así que lo podemos quitar.
Otra cuestión interesante con Rules es que el método applies solo se usa internamente, por lo que podemos quitarlo de la interfaz y hacerlo privado.
public interface Rule {
GameScore gameScore(int score1, int score2);
}
En las clases implementadoras quedaría así;
class TieRule implements Rule {
private final Rule delegate;
public TieRule(Rule delegate) {
this.delegate = delegate;
}
private boolean applies(int score1, int score2) {
return score1 == score2;
}
@Override
public GameScore gameScore(int score1, int score2) {
if (applies(score1, score2)) {
return new TieScore(score1);
}
return delegate.gameScore(score1, score2);
}
}
De nuevo, todos estos cambios nos los permite consolidar el flujo TCR. De hecho, nos deja quitar un poco más de código en DefaultRule:
class DefaultRule implements Rule {
@Override
public GameScore gameScore(int score1, int score2) {
return new NormalScore(score1, score2);
}
}
Al revisar el código en general, vemos que hay alguna parte que todavía está un poco fea, es decir, el código parece alambicado y no comunica nada bien qué es lo que ocurre. Nos podemos hacer una idea, pero posiblemente se podría expresar todo mejor. Helo aquí:
class NormalScore implements GameScore {
private int m_score1;
private int m_score2;
public NormalScore(int m_score1, int m_score2) {
this.m_score1 = m_score1;
this.m_score2 = m_score2;
}
@Override
public String score() {
String score = "";
int tempScore = 0;
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
switch (tempScore) {
case 0:
score += "Love";
break;
case 1:
score += "Fifteen";
break;
case 2:
score += "Thirty";
break;
case 3:
score += "Forty";
break;
}
}
return score;
}
}
En resumidas cuentas, lo que hace este bloque es traducir la puntuación numérica de cada jugadora al sistema de puntuación del tenis. La forma de hacerlo es realmente extraña y compleja. Vamos a ver cómo podemos refactorizarlo. Primero extraemos un método scoreToString:
class NormalScore implements GameScore {
private int m_score1;
private int m_score2;
public NormalScore(int m_score1, int m_score2) {
this.m_score1 = m_score1;
this.m_score2 = m_score2;
}
@Override
public String score() {
String score = "";
int tempScore = 0;
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
score = scoreToString(score, tempScore);
}
return score;
}
private String scoreToString(String score, int tempScore) {
switch (tempScore) {
case 0:
score += "Love";
break;
case 1:
score += "Fifteen";
break;
case 2:
score += "Thirty";
break;
case 3:
score += "Forty";
break;
}
return score;
}
}
Este cambio compila y pasa los tests, por lo que es consolidado. Ahora lo que nos interesa es cambiar esta línea:
score = scoreToString(score, tempScore);
para que sea:
score += scoreToString(tempScore);
Para ello, necesitamos hacer esto en scoreToString:
private String scoreToString(String score, int tempScore) {
switch (tempScore) {
case 1:
return "Fifteen";
case 2:
return "Thirty";
case 3:
return "Forty";
default:
return "Love";
}
}
y ya podemos prescindir del parámetro score porque no lo necesitamos más:
@Override
public String score() {
String score = "";
int tempScore = 0;
for (int i = 1; i < 3; i++) {
if (i == 1) tempScore = m_score1;
else {
score += "-";
tempScore = m_score2;
}
score += scoreToString(tempScore);
}
return score;
}
private String scoreToString(int tempScore) {
switch (tempScore) {
case 1:
return "Fifteen";
case 2:
return "Thirty";
case 3:
return "Forty";
default:
return "Love";
}
}
Volveremos sobre el método scoreToString, pero ahora vamos a prestar atención al método score. En realidad, podría hacerse de una manera mucho más sencilla:
@Override
public String score() {
return scoreToString(m_score1) + "-" + scoreToString(m_score2);
}
Volviendo a scoreToString un HashMap debería ser más que suficiente en lugar de usar switch:
private String scoreToString(int tempScore) {
HashMap<Integer, String> points = new HashMap<Integer, String>();
points.put(0, "Love");
points.put(1, "Fifteen");
points.put(2, "Thirty");
points.put(3, "Forty");
return points.get(tempScore);
}
Este cambio pasa los tests. Sin embargo, la implementación en general podría ser un poco más limpia. Primero quitamos el método scoreToString.
@Override
public String score() {
HashMap<Integer, String> points = new HashMap<Integer, String>();
points.put(0, "Love");
points.put(1, "Fifteen");
points.put(2, "Thirty");
points.put(3, "Forty");
return points.get(m_score1) + "-" + points.get(m_score2);
}
Y ahora movemos la creación del HashMap al constructor. También arreglamos un poco el naming.
class NormalScore implements GameScore {
private final HashMap<Integer, String> points;
private final int score1;
private final int score2;
public NormalScore(int score1, int score2) {
this.score1 = score1;
this.score2 = score2;
points = new HashMap<Integer, String>();
points.put(0, "Love");
points.put(1, "Fifteen");
points.put(2, "Thirty");
points.put(3, "Forty");
}
@Override
public String score() {
return points.get(score1) + "-" + points.get(score2);
}
}
Y podríamos aplicar el mismo tratamiento en TieScore, puesto que funciona de manera bastante parecida. Primer paso:
class TieScore implements GameScore {
private final int score;
TieScore(int score) {
this.score = score;
}
@Override
public String score() {
HashMap<Integer, String> points = new HashMap<Integer, String>();
points.put(0, "Love-All");
points.put(1, "Fifteen-All");
points.put(2, "Thirty-All");
return points.getOrDefault(score, "Deuce");
}
}
Ejecutamos TCR para ver que se mantienen los cambios y colocamos un poco mejor:
class TieScore implements GameScore {
private final int score;
private final HashMap<Integer, String> points;
TieScore(int score) {
this.score = score;
points = new HashMap<Integer, String>();
points.put(0, "Love-All");
points.put(1, "Fifteen-All");
points.put(2, "Thirty-All");
}
@Override
public String score() {
return points.getOrDefault(score, "Deuce");
}
}
El resultado es que tenemos el código mucho mejor estructurado, con muchos objetos que se encargan de tareas específicas y que funcionan cooperativamente. Hay muchos detalles que podrían mejorarse, pero creo que como ejemplo de la técnica está bastante bien.
Conclusiones: TCR for the win
Creo que el flujo TCR, combinado con técnicas como Approval Test (o Golden Master) para generar tests que proporcionen alta cobertura, me parece ideal para refactorizar código de producción. El mayor beneficio es que contribuye a garantizar que únicamente introducimos cambios desplegables, de una forma sistemática y en pasos pequeños y controlados.