Meditando un test de código.

Hace poco he realizado un test de código y he discutido los pormenores de la implentación con otros desarrolladores de software. Como en medio de una conversación en tiempo real siempre es más complicado estructurar tus pensamientos y materializar en palabras lo que la experiencia en un tema te da; en forma de "feeling in the guts"; voy a poner aquí mi posicionamiento de la implementación al respecto para estructurar mis propios pensamientos y mejorar mis dotes debatidoras, que son un poco patéticas y de paso puede interesarle a algún lector.

El código es el cálculo de una factura de un contratista de reformas. Para el cálculo de la factura existen reglas de tarificación según el tiempo invertido y además tenemos un descuento en el trabajo más largo.

Voy poniendo el código y explicando que hace y porqué he tomado tales decisiones:

Lo primero que hice fue un procesador de los items de trabajo. Este procesador usa una cadena de entrada de esta guisa: "fontaneria;02:05:35\npintura;00:35:45\nfontaneria;01:00:00". Donde "\n" separa items y ";" separa datos del item. Aquí presenta 2 trabajos de fontanería con sus horas, minutos y segundos invertidos y uno de pintura.

public interface JobParser
{
 IEnumerable<(string, TimeSpan)> Jobs();
}

public class StringJobParser : JobParser
{
 private char dataSeparator;
 private string[] jobLines;
 
 public StringJobParser(string jobList, char lineSeparator, char dataSeparator)
 {
  this.jobLines = jobList.Split(lineSeparator);
  this.dataSeparator = dataSeparator;
 }
 
 IEnumerable<(string, TimeSpan)> JobParser.Jobs() //return list of tuples
 {
  foreach (string call in jobLines)
  {
   string[] jobData = call.Split(dataSeparator);
   yield return (jobData[0], TimeSpan.Parse(jobData[1])); //lazy loop to avoid O(N+M)
  }
 }
}

Análisis de las decisiones tomada:

• Una interfaz para que el que use el parser no tenga que conocer los detalles del origen de los datos ni como se procesan y una clase; ya que necesitamos estado. (dataSeparator)
• Un constructor que permite configurar los separadores de la cadena y el source de los datos para tener flexibilidad en caso de que cambien en un futuro y poder inyectarle un source falso para tu testeabilidad.
• Interfaz de retorno que permite que el parser utilice una gran variedad de colecciones diferentes sin necesidad de cambiar la firma de la función.
• Un tipo ValueTuple de elementos de la colección. Independiza la salida para no tener que cambiar la firma de la función aunque la implementación del parser cambie. También libramos de presión al GC ya que este tipo es un tipo de valor; no de referencia y nos libramos de crear instancias de clases de vida muy corta que pueden llegar a no necesitarse (ver el agregador más adelante).
• Un loop en diferido. Que nos ahorra un recorrido extra para procesar todas la líneas de trabajo y luego otro para calcular el precio de la factura. También quita presión a la memoria puesto que no necesitamos tener la lista tuplas en memoria toda entera a la vez para que el consumidor las procese.
• Esta implementación también nos permitiría saltar a programación reactiva casi sin impacto, puesto que simplemente hacer un toObservable al IEnumerable y suscribirse a él ya nos bastaría. Esto sería cosa buena en caso de que el parser realizase algún proceso asíncrono para la obtención de items de trabajo. Y el consumidor de la interfaz ni se enteraría ni le importaría. Canela fina!
• La responsabilidad única del parser lo hace trivialmente testeable.

¿Mucha chicha para una docena de líneas de código verdad? :-P

Después hice una función que se encarga de calcular los precios por segundos:

Func<TimeSpan, uint> priceCalculator = (duration) =>
 {
  return (uint)(duration.TotalMinutes > 5 ? duration.TotalSeconds * 3 : duration.TotalSeconds * 5);
 };

Análisis de las decisiones tomadas:

• Una función porque no necesitamos estado. ¡Inmutabilidad y programación funcional al poder! Anda que no es fácil testear esta función...
• Independencia del calculador de la factura. ¿Que tienen que trabajar los fines de semana y es el doble de caro? Pues otra función multiplicando por el doble y arreando. Y sigue siendo super fácil testear todas las funciones diferentes que necesitemos para los cálculos de precios.
• Si esto no es responsabilidad única no sé que más puede ser...

Luego un calculador de facturas que utiliza el parser y la función de calcular el precio de cada trabajo para dar el precio final de la factura aplicando las reglas negociadas (un descuento del trabajo más largo en este caso)

public interface BillCalculator
{
 uint CalcBillPrice();
}

public class FreeLongestJobToSameJobTypePlan : BillCalculator
{
 private Func<TimeSpan, uint> jobPriceCalculator;
 private JobParser cp;
 
 public FreeLongestJobToSameJobTypePlan(JobParser cp, Func<TimeSpan, uint> jobPriceCalculator)
 {
  this.cp = cp;
  this.jobPriceCalculator = jobPriceCalculator;
 }
 
 //use a dictionary to aggregate jobs duration and prices and select longest duration job to same job type to subtract from bill price
 public uint CalcBillPrice()
 {
  Dictionary<string, AggregatedJob> jobAggregation = new Dictionary<string, AggregatedJob>();
  AggregatedJob selectedFreeJobs = AggregatedJob.Empty();
  uint billPrice = uint.MinValue;
 
  AggregatedJob currentAggregatedjob;
 
  //use job parser to retrieve every job data
  foreach ((string job, TimeSpan duration) in cp.Jobs())
  {
   var price = callPriceCalculator(duration);
   billPrice += price; //sum job prices for bill
 
   if (jobAggregation.TryGetValue(job, out currentAggregatedjob))
   {
    //a job to this job type already exist
    currentAggregatedjob.addJobData(duration, price); //add duration and price to this job numb
   }
   else
   {
    //a job to this job type does not exist
    currentAggregatedjob = new AggregatedJob(job, duration, price); //create new one
    jobAggregation.Add(currentAggregatedjob.jobType, currentAggregatedjob); //add to the aggregation
   }
 
   selectedFreeJobs = selectCurrentFreeJobs(selectedFreeJobs, currentAggregatedjob); //check if we must change the selected for free
  }
 
  return billPrice - selectedFreeJobs.Price; //return billPrice minus jobs selected to be free
 }

private AggregatedJob selectCurrentFreeJobs(AggregatedJob selected, AggregatedJob otherJob)
 {
  if (areSame()) return selected;
 
  if (sameDuration())
  {
   return aphabeticalyCompareGreaterThan(selected.jobType, otherJob.jobType) ? selected : otherJob; //select minimun jobType value
  };
 
  return otherJob.Duration > selected.Duration ? otherJob : selected;
 
  bool areSame()
  {
   return ReferenceEquals(selected, otherJob);
  }
 
  bool sameDuration()
  {
   return otherJob.Duration == selected.Duration;
  }
 }

}

Análisis de las decisiones tomadas:

• Una interfaz para que se puedan implementar otros calculadores de facturas con otros descuentos y/o reglas y se puedan usar independientemente.
• Una clase, ya que necesitamos estado; siempre asumiendo que nos interesaría cambar el calculador de precios o el origen de los datos sin tener que crear una nueva instancia del calculador.
• Testeabilidad ya que podemos pasar falsos orígenes de datos y calculadores de precio fácilmente para tener su salida controlada.
• Baja presión al GC. Solo creamos un nuevo nodo para el diccionario cuando es necesario.
• La función selectCurrentFreeJobs no tiene estado lo que la hace fácilmente testeable a través de una clase proxy que exponga la función al público. (Los proxies para test no me convencen e intento evitarlos pero a veces puede venir bien)
• Aquí hay una ligera pega y es que en vez de aplicar la premisa "tell, dont ask"; estoy accediendo a la duración de una agregación de trabajos para compararlas. Esto se puede hacer en este contexto porque, como la agregación de trabajos es una clase que debe ser interna al calculador de facturas, (es un detalle de implementación especifica de esta clase en concreto)  no se filtra a otras subclases o a otros implementadores de la interfaz BillCalculator.

Recordemos que muchos de los patrones para una buena OOP son para evitar que los detalles de implementación se filtren al resto de componentes así que siempre se pueden hacer, meditádamente y con sentido común, pequeñas concesiones para simplificar el código.

Y una ultima cosa: Favorecer la composición sobre la herencia. Como se puede apreciar aquí, podemos cambiar la implementación y comportamiento de cada componente sin tener que recurrir a herencia. Lo que aporta mayor flexibilidad.

Y esto es, a bote pronto; que tampoco me he pegado una panzada analizando, lo que se me ocurre cuando intento materializar el "feeling in the guts" de mis años programando en palabras.

¿Ruegos, lloros, preguntas, críticas? Abajo en los comentarios sois bienvenidos.

Happy coding bastardillos!