Feeds:
Entradas
Comentarios

Archive for the ‘Refactor’ Category


Después del interludio con el spring io, vuelvo a la carga con el tema del diseño ágil de software. Como ya comenté, considero que dentro de las limitaciones de tiempo y dinero de un proyecto, hay que estar continuamente reestructurando el código con el objetivo de que sea fácil y barato cambiarlo cada vez que se produzca un cambio funcional, añadamos un nuevo requisito o necesitemos arreglar un bug. A estas reestructuraciones de código se les llama “refactors”.

Un refactor es una transformación de código que no altera la funcionalidad de este, pero sí su estructura. La idea es hacer refactors que hagan que nuestro código sea más flexible y maleable para poder alterarlo de forma rápida. El truco está en saber si al aplicar un refactor estamos mejorando nuestro código o si simplemente lo estamos complicando ¿Qué criterios sigo yo para evaluar si un refactor es bueno o no? Básicamente sólo sigo los tres siguientes:

  • Que el código resultante sea más legible.  Como ya expliqué considero este el criterio más importante.
  • Que se respeten los principios DRY y “experto en información”. Ya los explicaré en otro post
  • Que se respeten los principios SOLID. Entre este post y los siguientes pienso ir explorándolos.

¿Qué es el SOLID? SOLID es un acrónimo que consolida 5 principios: Single responsability, Open/Closed, Liskov substitution, Interface segregation y Dependency inversion. En este post me ocuparé del principio de única responsabilidad (Single responsability o SRP), o sea la S  de SOLID.

El principio de única responsabilidad (SRP) simplemente nos dice que ningún artefacto de código debe tener más de una única responsabilidad, y por lo tanto debe implementar una única funcionalidad. El principio aplica a sistemas, aplicaciones, frameworks, objetos y métodos. Obviamente según el artefacto al que se aplique, la amplitud del concepto de única responsabilidad cambia. Pero en este post nos vamos a centrar en cómo afecta a nuestra clases y objetos.

¿Cómo sabemos si un artefacto, como una clase o un método, respeta este principio? Es muy sencillo, simplemente  debemos preguntarnos a nosotros mismos sobre cuantas razones diferentes podemos tener para querer cambiar dicho artefacto. Un artefacto que cumple el principio de única responsabilidad sólo puede ser cambiado debido a una única razón. Si encontramos múltiples razones para cambiar un artefacto entonces no cumple dicho principio y debe ser refactorizado. A mi me gusta llamar a este principio, el principio del bombero/torero. El hecho de que el personaje de bombero/torero nos cause risa es que es totalmente ridículo que una misma persona realice tareas de dos profesiones tan diferentes al mismo tiempo. Si lo vemos absurdo en una persona, ¿porque nos quedamos tan tranquilos cuando vemos una clase que es un bombero/torero?

Para entender mejor este principio veamos un ejemplo. Suponed que tenemos una historia de usuario tal que sigue:

Cuando se realiza una transferencia importante

Entonces se genera un mensaje con información sobre dicha transferencia

Y se notifica a las personas de interés

Para que den su aprobación y así evitar el fraude fiscal”

Hablando con el cliente me dice que la persona de interés es un auditor, y que había pensado en notificarle vía mail. Las transferencias importantes son aquellas por encima de 1000 euros (son un poco ratas). Cómo estoy codificando en JAVA y usando el famoso framework Zprin, el código que me sale es el siguiente:

public class AuditorTransferenciasMonetarias {
  // Inyección de dependencias con el famoso framework Zprin (TM)
  // Con este framework somos productivos de la leche
  private ZprinPropertySource systemConfiguration;
  private ZprinTemplateEngine templateEngine;
  private ZprinMailIt mailIt;

  public void transferenciaRealizada(Transferencia transferencia) {
    if(this.esTransferenciaImportante(transferencia)) {
      String auditor=this.obtenerDireccionMailAuditor();
      String mensaje=this.componerMensajeAviso(transferencia);
      ConexionMail conexionMail=null;
      try {
        conexionMail=this.abrirConexionMail();
        conexionMail.enviar(new Mail().to(auditor).withBody(mensaje));
      } finally {
        if(conexionMail!=null)
          conexionMail.cerrar();
      }
    }
  }
  private boolean esTransferenciaImportante(Transferencia transferencia) {
    return transferencia.importe()>1000;
  }
  private String obtenerDireccionMailAuditor() {
    return this.systemConfiguration.getProperty("auditor.email");
  }
  private String componerMensajeAviso(Transferencia transferencia) {
    return this.templateEngine.getTemplate("aviso-transferencia-importante.ztpl").execute(transferencia);
  }
  private MailConnection abrirConexionMail() {
    return this.mailIt.openConnectionTo(this.systemConfiguration.getProperty("servers.mail"));
  }
}

Buen código, al usar el framework soy muy productivo y hago la clase super rápido, es más me quito de encima problemas complejos como el envío de mails, el acceso a propiedades configurables, o el tener que hacerme un motor de plantillas :-P Sólo tiene un problema, que no cumple el SRP (maldito SOLID, quién lo habrá inventado). SRP nos dice que la clase sólo debe tener una responsabilidad y la única manera en la que esta debería tener que cambiarse es cuando hay alguna alteración en dicha responsabilidad. En este caso la responsabilidad viene definida por la historia de usuario, así que mientras esta no cambie de forma significativa, entonces no deberíamos vernos forzados a cambiar la implementación. Obviamente el código descrito arriba no cumple esta propiedad. Vayamos por partes.

Suponed que el criterio para clasificar a una transferencia como importante cambia. Nos veríamos forzados a tocar nuestra clase, aunque realmente nuestra historia de usuario es la misma. Para evitarlo alteramos nuestro código:

public class AuditorTransferenciasMonetarias {
  // ....
  public void transferenciaRealizada(Transferencia transferencia) {
    if(transferencia.esImportante()) {
      String auditor=this.obtenerDireccionMailAuditor();
      String mensaje=this.componerMensajeAviso(transferencia);
      ConexionMail conexionMail=null;
      try {
        conexionMail=this.abrirConexionMail();
        conexionMail.enviar(new Mail().to(auditor).withBody(mensaje));
      } finally {
        if(conexionMail!=null)
          conexionMail.cerrar();
      }
    }
  }
  // ....
}

Como vemos ahora la clase Transferencia tiene un método que nos indica si es importante o no. Si el criterio de importancia cambia nos da igual, ya que este método nos protege de dicho cambio. Si el cliente decide que quiere un motor de reglas parametrizable en runtime mediante un backoffice, el afectado es Transferencia no la clase que estamos construyendo.

Otro motivo de cambio ajeno a nuestra responsabilidad es que la definición de “personas interesadas” cambie. De repente queremos notificar no sólo a un auditor, sino a varios y además al director de la sucursal. Para protegernos de ello veamos el siguiente cambio:

public class AuditorTransferenciasMonetarias {
  // ....
  // Ahora tenemos un colaborador de "negocio"
   // que está al mismo nivel de abstracción que esta clase
  private DirectorioEmpleados directorioEmpleados;

  public void transferenciaRealizada(Transferencia transferencia) {
    if(transferencia.esImportante()) {
      String mensaje=this.componerMensajeAviso(transferencia);
      ConexionMail conexionMail=null;
      try {
        conexionMail=this.abrirConexionMail();
        conexionMail.enviar(rellenarDestinatarios(new Mail().withBody(mensaje)));
      } finally {
        if(conexionMail!=null)
          conexionMail.cerrar();
      }
    }
  }
  private Mail rellenarDestinatarios(Mail mail) {
    for(Empleado interesado: this.interesadosEnTransferenciasImportantes())
      mail=mail.to(interesado.email());
    return mail;
  }
  private List<Empleado> interesadosEnTransferenciasImportantes() {
    return this.directorioEmpleados.empleadosConRol(Empleado.INTERESADOS_TRANSFERENCIAS_IMPORTANTES);
  }
  // ....
}

Ahora gracias a la interface DirectorioEmpleados, podemos buscar a todos los empleados que necesitamos notificar. Para ello buscamos por rol. La asociación empleado/rol se puede definir aparte, e incluso ser dinámica y cambiar en runtime. Observad que hemos añadido un colaborador que está al mismo nivel de abstracción de la clase que estamos codificando, y que el código ahora se parece un poco más a la definición de la historia de usuario.

Otro motivo por el cual podemos vernos obligados a cambiar esta clase, es que ahora los mensajes no se envíen por mail, sino por SMS. O peor, ¡ que se envíen a la vez por SMS, Mail, y chat a la vez ! Como no queremos que nos pillen con eso (que me conozco al cliente, que cambia de opinión constantemente) volvemos a “mejorar” el código:

public class AuditorTransferenciasMonetarias {
  // ...
  private SistemaMensajeriaCorporativa mensajero;

  public void transferenciaRealizada(Transferencia transferencia) {
    if(transferencia.esImportante()) {
      this.mensajero
            .enviar(
              new Mensaje()
                   .a(this.interesadosEnTransferenciasImportantes())
                   .conContenido(this.componerMensajeAviso(transferencia))
      );
    }
  }
  // ....
}

Esta vez hemos introducido el sistema de mensajería corporativa, que nos permite enviar un mensaje de forma abstracta a varios empleados. Ya está, ya no hay más motivos de cambio… no perdón nos queda otro motivo y gordo. Hablando con nuestro amigo el “friki”, nos cuenta que el framework Zprin ya no es el acabose y que ha salido el framework XprinGio, y que a Zprin se queda sin soporte en menos de dos años. Nos olemos la tostada, en cuanto se entere el arquitecto jefe del temita nos manda cambiar de framework, o peor, si se entera el jefe de proyecto que sólo nos quedan dos años de soporte, toma migración. Hay que protegerse a toda costa de tan temida migración:

public class AuditorTransferenciasMonetarias {
  private DirectorioEmpleados directorioEmpleados;
  private SistemaMensajeriaCorporativa mensajero;
  private PlantillasCorporativas almacenDePlantillas;

  public void transferenciaRealizada(Transferencia transferencia) {
    if(transferencia.esImportante()) {
      this.mensajero
            .enviar(
              new Mensaje()
                   .a(this.interesadosEnTransferenciasImportantes()
                   .conContenido(this.componerMensajeAviso(transferencia))
      );
    }
  }
  private Documento componerMensajeAviso(Transferencia transferencia) {
    return this.almacenDePlantillas.usandoPlantilla("avisos/transferencia-importante").crearDocumentoCon(transferencia);
  }
  private List<Empleado> interesadosEnTransferenciasImportantes() {
    return this.directorioEmpleados.empleadosConRol(Empleado.INTERESADOS_TRANSFERENCIAS_IMPORTANTES);
  }
}

Esta vez hemos eliminado todo dependencia con Zprin, y tenemos un motor de plantillas abstracto. Obviamente cuando implementemos el motor de plantillas, o el sistema de mensajería nos acoplaremos a Zprin, pero en una posible migración, sólo tendremos que cambiar estos servicios y no todas las clases de negocio. Otro cambio es que ahora la plantilla genera un Documento en vez de un String. Además la clase Documento sabrá serializarse en cualquier formato que se necesite por parte del mecanismo de mensajería (String, XML, HTML, JSON, PDF…) lo que permite tener sistemas de mensajería más complejos.

Como se observa, si seguimos el principio de única responsabilidad nuestros artefactos tendrán una alta cohesión y por lo general un bajo acoplamiento, ya que quedan pocos métodos muy relacionados entre si y muy cortos. Pero lo mejor es que se tenderá a que la tasa de cambios de un único artefacto baje, y que éstos cambios sean más localizados, ya que sólo se va a necesitar modificarlo ante un único tipo de cambio al existir una relación uno a uno entre responsabilidad y clase. Obviamente este era un ejemplo de código algo artificial, y seguro que le sacais pegas, pero creo que podéis captar la idea.

Pero no es oro todo lo que reluce, más adelante en esta serie veremos el principio DRY y el “experto en información” y como ambos se terminan pegando con el principio de única responsabilidad, con lo que se produce un conflicto grave. Para colmo, el principio de segregación de interfaces nos puede complicar también la vida. Armonizar todos estos principios, es el caballo de batalla del diseño orientado a objetos. De hecho, respetar cada principio por separado es sencillo, lo difícil es juntarlos todos, ya que veréis que es como si unos principios contradijeran a otros ¿Terminará siendo la OO en realidad un paradigma fracasado y contradictorio?

Read Full Post »


Andaba yo buscando un ejemplo de código para demostrar el concepto del TDD, refactor y diseño emergente, cuando mi señora me apuntó al coding dojo en Ruby, organizado por @madridrb el pasado día 30 de Diciembre, donde durante las cervezas post-evento se me ocurrió este post. El dojo, donde el señor @ecomba propuso hacer la kata de los números romanos, se encontraba con bastante más gente de lo normal, y es que este hombre tiene mucho tirón. La idea era ir turnándose cada 3 minutos en un ordenador e ir avanzando la kata. Como éramos bastantes, y no había mucho tiempo, al final realmente no pudimos avanzar lo suficiente, y tuvo que acabarla @ecomba de forma rápida y sin poder entretenerse mucho. Esto hizo que tuviera que dar algunos saltos de diseño. Posteriormente en las cervezas me enteré que muchos asistentes no estaban muy versados en esto del TDD y que estaban confusos. Sobre todo la mayor discusión trataba sobre por qué era importante hacer TDD y refactor en pasos muy pequeños y qué ventaja daba a la hora de diseñar el código. Así que en ese momento supe que mi siguiente post iba a ser un ejemplo con código sobre cómo conseguir un diseño emergente a base de Refactor y TDD en pasos pequeños (baby steps).

Este es uno de los posts más difíciles que he hecho ya que voy a intentar mostrar mis procesos mentales de diseño, y como me enfrento a un problema de programación (sencillo por cierto). Aviso a los asistentes del coding dojo que el código final al que llego en este post es diferente al que hizo @ecomba, lo que demuestra que no suele existir una única implementación “óptima”, y que el resultado puede variar en función de criterios personales y algo del azar. No tengo experiencia alguna en Ruby, sólo he hecho los koans y esta kata, con lo que he usado un Ruby muy básico y sencillo al no conocer bien el lenguaje y la librería de objetos. Sin embargo guiado por el TDD y el refactor creo que llego a una solución bastante compacta. Empecemos…

Lo primero de todo es familiarizarse con la funcionalidad a implementar. Ni el TDD ni el refactor son un sustituto de ponerse a pensar, sólo un guía para nuestro proceso creativo, por lo que debemos tener alguna idea del objetivo funcional que debe alcanzar nuestro código. No es necesario ser un experto en la funcionalidad, sino saber por donde van más o menos los tiros y no ir dando palos de ciego. Aquí va la funcionalidad de los números romanos, al menos lo que recordaba cuando me puse a tirar código:

  • Queremos convertir un número entero en un número romano. Nada más.
  • Los romanos no representaban de forma explícita ni el cero ni los números negativos.
  • Existen un conjunto de 13 símbolos o numerales básicos (nosotros tenemos 10). Cada uno de estos tiene un valor predefinido, pero ninguno representa el 0 o valor negativo.
  • Para representar un número, se concatenan estos numerales, y sus valores se van sumando hasta que se obtiene el valor del número.
  • Algunos casos como el 4 o el 9 son especiales y se representan de forma especial.

Así que llegué a mi casa, abrí el vim, me instalé el rspec y secuestré el “mug of vi” de mi mujer para que me sirviera de chuleta (soy penoso con el vi y quería aprender, que lo pasé muy mal en el dojo). Lo primero que se hace siempre es coger el caso más básico, y que cosa más básica que el número 1. En rspec queda:

describe "Roman number" do
	it "I is equivalent to 1" do
		1.to_roman.should == 'I'
	end
end

Evidentemente falla, con lo que hay que añadir una implementación. Pero siguiendo las reglas del TDD y refactor, debe ser la implementación más simple posible que pase el test, ya lo pondremos bonito después si existiera una razón de peso. Para los que no sepan Ruby, en éste lenguaje las clases son abiertas, por lo que el enfoque de diseño tomado es añadir un método “to_roman” a la clase “Fixnum” que representa a los enteros.  La implementación es obvia:

class Fixnum
	def to_roman
		'I'
	end
end

describe "Roman number" do
	it "I is equivalent to 1" do
		1.to_roman.should == 'I'
	end
end

Vamos a probar ahora con el número 5, que es otro de los numerales básicos romanos. La estrategia de momento es ir añadiendo numerales básicos, a ver que nos sale. El test y su correspondiente implementación:

class Fixnum
	def to_roman
		return 'V' if self == 5
		'I'
	end
end

describe "Roman number" do
	it "I is equivalent to 1" do
		1.to_roman.should == 'I'
	end

	it "V is equivalent to 5" do
		5.to_roman.should == 'V'
	end
end

La técnica es sencilla, copiar y pegar lo que funcionó en el caso del 1. Reemplazamos los valores adecuados y cubrimos cada caso con un if. Estamos en modo “pasar tests”, hasta el momento no hemos detectado duplicación y no hemos refactorizado nada. Pero esto ya empieza a oler a cuerno quemado. Volvemos a repetir, esta vez para el 10. La cosa queda:

class Fixnum
	def to_roman
		return 'X' if self == 10
		return 'V' if self == 5
		'I'
	end
end

describe "Roman number" do
	it "I is equivalent to 1" do
		1.to_roman.should == 'I'
	end

	it "V is equivalent to 5" do
		5.to_roman.should == 'V'
	end

	it "X is equivalent to 10" do
		10.to_roman.should == 'X'
	end
end

Ya se observa claramente la duplicación. Tenemos dos (o tres, según se mire) lineas con la misma estructura de código. Esto es normal ya que hemos estado haciendo copy&paste. Ahora toca reflexionar sobre la intención de nuestro código, ¿qué queremos hacer en realidad? ¿Refleja el código actual esa intención (es expresivo)? Es obvio que lo que queremos hacer es mapear números a numerales romanos, y que existe una correspondencia uno a uno. En realidad estamos haciendo una búsqueda de un literal romano por número entero a base de un montón de ifs. El uso de una simple Hash (o mapa o diccionario) nos elimina las líneas duplicadas y nos da un código más expresivo (si elegimos bien los nombres). Tras refactorizar me sale lo siguiente:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 5 => 'V', 1 => 'I' }

	def to_roman
		ARABIC_TO_ROMAN_NUMERAL[self]
	end
end
# .......

Por brevedad no me entretendré con los siguientes 10 numerales romanos, y pasaré al siguiente problema, ¿qué pasa si un número no se corresponde con un literal romano? Según el funcional hay que concatenar los numerales hasta sumar el número deseado. Como caso más sencillo de este escenario añado el test para el número 2 que debe transformarse en ‘II’. Además voy a refactorizar un poco los ejemplos de test, agrupándolos por escenario. En RSpec usaré un contexto por escenario (no se si esto es purista pero a mi me parece bien). El test ahora es:

# .........
describe "Roman number" do
	context "has basic numerals with different values" do
		it "I is equivalent to 1" do
			1.to_roman.should == 'I'
		end

		it "V is equivalent to 5" do
			5.to_roman.should == 'V'
		end

		it "X is equivalent to 10" do
			10.to_roman.should == 'X'
		end
	end

	context "concatenates numerals in descending order until they sum up the desired integer" do
		it "II is equivalent to 2" do
			2.to_roman.should == 'II'
		end
	end
end

Como veis un contexto para los numerales básicos y otro para los que no lo son. Añado la implementación más básica que se me ocurre, si el número coincide con un numeral básico lo devuelvo y acabo, si no, devuelvo ‘II’ a cascoporro:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 5 => 'V', 1 => 'I' }

	def to_roman
		return ARABIC_TO_ROMAN_NUMERAL[self] if ARABIC_TO_ROMAN_NUMERAL[self]
		'II'
	end
end
# ........

Añado un test para el número 3, que debe resultar en ‘III’ y su correspondiente implementación. Pero esta vez me lo curro un poco más:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 5 => 'V', 1 => 'I' }

	def to_roman
		return ARABIC_TO_ROMAN_NUMERAL[self] if ARABIC_TO_ROMAN_NUMERAL[self]
		'I' + (self - 1).to_roman
	end
end

describe "Roman number" do

#      ......................

	context "concatenates numerals in descending order until they sum up the desired integer" do
		it "II is equivalent to 2" do
			2.to_roman.should == 'II'
		end

		it "III is equivalent to 3" do
			3.to_roman.should == 'III'
		end
	end
end

Como veis esta vez he sido más sofisticado y en vez de meter un ‘III’ if self == 3, he leído bien el funcional y se me ha ocurrido un algoritmo. Siguiendo el espíritu de concatenar hasta sumar el número, se me ocurre que puedo tomar el numeral ‘I’ y restarle su valor al número que quiero convertir. El resultado de esta resta, qué es lo que queda para conseguir sumar el número deseado, lo convierto a su vez en un número romano y lo concateno. Aquí hemos tenido que parar y reflexionar sobre la funcionalidad para obtener una idea creativa. El TDD nos ha servido para llegar a un punto donde esta idea se nos pueda ocurrir con facilidad.

Sigamos, ¿qué pasa con otros casos? Añado tests para el 6, el 11 el 15 y el 20. Los hago pasar haciendo copy&paste del caso del 2 y el 3, reemplazando ‘I’ y 1, por ‘V’ y 5 para pasar el test del 11, y ‘X’ y 10 para pasar el test del número 9. La cosa queda así:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 5 => 'V', 1 => 'I' }

	def to_roman
		return ARABIC_TO_ROMAN_NUMERAL[self] if ARABIC_TO_ROMAN_NUMERAL[self]
		return 'X' + (self - 10).to_roman if self > 10
		return 'V' + (self - 5).to_roman if self > 5
		'I' + (self - 1).to_roman
	end
end

describe "Roman number" do

#      .................

	context "concatenates numerals in descending order until they sum up the desired integer" do
		it "II is equivalent to 2" do
			2.to_roman.should == 'II'
		end

		it "III is equivalent to 3" do
			3.to_roman.should == 'III'
		end

		it "VI is equivalent to 6" do
			6.to_roman.should == 'VI'
		end

		it "XI is equivalent to 11" do
			11.to_roman.should == 'XI'
		end

		it "XV is equivalent to 15" do
			15.to_roman.should == 'XV'
		end

		it "XX is equivalent to 20" do
			20.to_roman.should == 'XX'
		end
	end
end

Fijaros en el orden en que pongo las líneas de código, primero evalúo los numerales con valor más alto y después las de valor más bajo. Si lo hacemos en otro orden los tests fallan, al devolverme por ejemplo ‘VVI’ en vez de ‘XI’. En este caso los tests son los que nos han hecho darnos cuenta de que hay que ordenar los numerales de mayor a menor, y además de sumar el valor, debe ser la representación más corta posible. Esto no lo tenía yo nada claro por el funcional que indiqué más arriba. En este caso los tests nos aclaran la funcionalidad.

Sin embargo de nuevo tenemos duplicación y el código da repelús. La misma estructura de código repetida en tres líneas, sólo varía en el numeral romano usado en cada caso y su correspondiente valor numérico. ¿Podemos sustituir esta duplicación por una regla parametrizable? ¿Quizás un método auxiliar que recibiera como parámetros el numeral romano y el valor numérico? Esta solución eliminaría algo de duplicación, pero aun quedaría duplicada la estructura de la cascada de ifs. Es esto último lo que más me preocupa, ya que se viola el principio abierto/cerrado. Cuando quisiéramos añadir un nuevo numeral (cuando nuestro cliente recordara uno nuevo), tendríamos que “abrir” el método to_roman para añadir otra línea más. Esto es más grave que unos cuantos caracteres repetidos. Hay que pararse a pensar de nuevo, es hora de ganarse el sueldo. Lo que se me ocurrió es lo siguiente:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 5 => 'V', 1 => 'I' }

	def to_roman
		return ARABIC_TO_ROMAN_NUMERAL[self] if ARABIC_TO_ROMAN_NUMERAL[self]
		ARABIC_TO_ROMAN_NUMERAL.each do | arabic_number, roman_numeral |
			return roman_numeral + (self - arabic_number).to_roman if self > arabic_number
		end
	end
end
#  .............

La idea es recorrer la colección de numerales, de forma que encontremos el numeral romano de más valor, que sea inferior al número que buscamos, y usamos dicho numeral en la regla recursiva que descubrimos antes para calcular el resultado. Podríamos haber usado un bucle for, pero sospecho que no están bien vistos en el mundo de Ruby, así que uso el método iterador each, y le paso un bloque de código. En cuanto encuentro el numeral buscado devuelvo el valor y el bucle (perdón, la iteración) termina. De paso ya no necesito ese método auxiliar que se me ocurrió antes, ya que la regla recursiva sólo se usa una vez y es bastante compacta. Me encanta cuando los planes salen bien… WTF! ¡ Fallan los tests ! ¡ Qué c*** pasa ! ¡ Es la hora del debugger !

Lo que ocurre es que el método each itera las entradas de la hash en el orden que le sale de las gónadas. Es una sorpresa, ya que según la documentación, debería iterarlos en el orden en que los añades a la hash. Ya estoy por mandar un bug a la comunidad de Ruby cuando descubro mi fallo. Estoy usando Ruby 1.8.x y la documentación es de la 1.9.x. Algo huele a quemado, busco un poco y efectivamente: originalmente el orden de iteración de las hash era aleatorio, pero a partir de la 1.9.x es por orden de inserción. Esto de añadir cambios que rompen la API y cambiar sólo el “minor version” no es buena idea.

Total, que tengo una implementación que supuestamente funciona en Ruby 1.9 (no lo he probado), pero no en 1.8. Algo he de hacer. De momento me calmo un poco, y me dedico a arreglar otra duplicación de código que me está matando, la primera línea del método to_roman. Lo que hago es lo siguiente:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 5 => 'V', 1 => 'I' }

	def to_roman
		return '' if self == 0
		ARABIC_TO_ROMAN_NUMERAL.each do | arabic_number, roman_numeral |
			return roman_numeral + (self - arabic_number).to_roman if self >= arabic_number
		end
	end
end
# ..............

Directamente elimino la línea y cambio self>arabic_number por self>=arabic_number. La idea es que si encuentro un numeral que sea exactamente igual al valor buscado, también puedo aplicar la regla recursiva. En este caso el resto es 0, que como sabemos no se representa en números romanos. Se soluciona devolviendo cadena vacía como caso base de la recursividad cuando el número es 0, eliminando la fea duplicación que teníamos como caso base.

Ahora ya puedo centrarme en hacerlo retrocompatible con Ruby 1.8. Simplemente ordeno de forma explícita la hash en orden descendiente (de mayor a menor). Un cambio trivial:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 5 => 'V', 1 => 'I' }.sort.reverse

# ................

end

Dicho sea de paso, esta ordenación me devuelve un array, con lo que en vez de una hash termino con un array de pares clave/valor en la constante ARABIC_TO_ROMAN_NUMERAL. Debido a que itero con el método each, y a que realmente había dejado de usar la búsqueda por clave, el cambio es transparente.

Ahora toca enfrentarse a la tercera fase de la especificación funcional: los casos especiales. Los romanos no escribían ‘IIII’ para el número 4, sino ‘IV’. Lo mismo con el 9 que se representa como ‘IX’ en vez de ‘VIIII’ (esta regla es para los números romanos clásicos, la versión más primitiva no la tenía). Añado estos ejemplos y obviamente los tests fallan. Tal vez tenga que hacer un algoritmo de simplificación, de modo que si hay tres numerales seguidos iguales los sustituya por la versión simplificada. Parece difícil. Antes de complicarme la vida, y por si cuela, se me ocurre añadir ‘IX’ y ‘IV’ a los numerales básicos. Sorprendentemente funciona, me olvido de algoritmos complicados. Finalmente añado algunos tests con números complicados, para asegurarme que todo funciona bien. El código final es:

class Fixnum
	ARABIC_TO_ROMAN_NUMERAL = { 10 => 'X', 9 => 'IX', 5 => 'V', 4 => 'IV', 1 => 'I' }.sort.reverse

	def to_roman
		return '' if self == 0
		ARABIC_TO_ROMAN_NUMERAL.each do | arabic_number, roman_numeral |
			return roman_numeral + (self - arabic_number).to_roman if self >= arabic_number
		end
	end
end

describe "Roman number" do
	context "has basic numerals with different values" do
		it "I is equivalent to 1" do
			1.to_roman.should == 'I'
		end

		it "IV is equivalent to 4" do
			4.to_roman.should == 'IV'
		end

		it "V is equivalent to 5" do
			5.to_roman.should == 'V'
		end

		it "IX is equivalent to 9" do
			9.to_roman.should == 'IX'
		end

		it "X is equivalent to 10" do
			10.to_roman.should == 'X'
		end
	end

	context "concatenates numerals in descending order until they sum up the desired integer" do
		it "II is equivalent to 2" do
			2.to_roman.should == 'II'
		end

		it "III is equivalent to 3" do
			3.to_roman.should == 'III'
		end

		it "VI is equivalent to 6" do
			6.to_roman.should == 'VI'
		end

		it "XI is equivalent to 11" do
			11.to_roman.should == 'XI'
		end

		it "XV is equivalent to 15" do
			15.to_roman.should == 'XV'
		end

		it "XX is equivalent to 20" do
			20.to_roman.should == 'XX'
		end
	end

	context "converts even complex examples (to gain more trust in our implementation)" do
		it "XVIII is equivalent to 18" do
			18.to_roman.should == 'XVIII'
		end

		it "XIX is equivalent to 19" do
			19.to_roman.should == 'XIX'
		end

		it "XXXVII is equivalent to 37" do
			37.to_roman.should == 'XXXVII'
		end
	end
end

Como veis el código es diferente al que propuso @ecomba. Curiosamente @ialcazar me mostró la solución de @cavalle que es bastante similar a la mía, seguramente porque ambos hemos optado por un enfoque recursivo. Eso sí, ninguna de las tres soluciones tiene métodos de más de 4 líneas de código ;-). Notad que he tomado dos decisiones de diseño importantes: me he decidido por un diseño recursivo, y los casos especiales, como ‘IV’ o ‘IX’, los trato como numerales básicos. ¿Qué ocurriría si hubiéramos decidido que los casos especiales no son numerales básicos? ¿A alguien le interesa explorar este camino?

¿Para que nos ha servido el TDD? En este caso nos ha servido para guiar el proceso de pensamiento. Añadiendo código poco a poco puedo detectar duplicación, violaciones de principios SOLID, y otros problemas rápidamente. Es en estos momentos donde TDD+Refactor en pasos pequeños, nos obliga a parar y pensar. Hasta que no tengamos una visión más profunda del problema, no podremos avanzar, y ésta forma de trabajar nos golpea en la cabeza obligándonos a reflexionar. Sin embargo no olvidemos que el problema de los números romanos es pura algorítmica. En casos más complejos como diseño OO el TDD brilla en todo su esplendor.

Si queréis, podéis entrar github y echarle un vistazo a todo el histórico de “baby steps” que fui haciendo. ¿Alguien se anima a hacerlo en otro lenguaje?

Read Full Post »

Seguir

Recibe cada nueva publicación en tu buzón de correo electrónico.

Únete a otros 43 seguidores