Ejemplo de un método deshonesto en c#.net y cómo refactorizarlo

Photo by Claudio Schwarz on Unsplash

Aquí tienes un ejemplo real de un método deshonesto en c# y un par de opciones para mejorarlo. 

Imagina que tu aplicación presenta un listado de facturas por pantalla y necesitas poder filtrarlo por zona geográfica.

Suponiendo que trabajas con EntityFramework, podrías generar un código como el siguiente:

public async Task<IActionResult> OnGetAsync()
{
    var consulta = _db.Facturas.AsQueryable();

    if(ZonaId.HasValue)
           consulta = consulta.Where(m=>m.Zona.Id == ZonaId);

    Facturas = await consulta.ToListAsync();

    return Page();
}

El parámetro ZonaId representa la zona seleccionada en la pantalla. El valor null de ZonaId representa el valor "zona" no seleccionada, para cuando el usuario no quiera filtrar por zona.

La primera versión de código ya es válida y podrías entregar la funcionalidad, pero hay un par de puntos mejorables:

• Hay un If en medio de las cláusulas LINQ que rompe la dinámica funcional antes de conseguir el resultado final y que además obliga a generar una variable "consulta". Sería deseable eliminar ese if.
   
• Por otro lado, podríamos encapsular el filtro Where y darle semántica. Eso ayudaría a entender mejor la intención del código y además a generar una "pieza de código" reutilizable.

Para solucionar estos dos puntos lo primero que se me ocurre es crear una método extensor como el siguinete.

public static class FiltrosFactras
{
  public static IQueryable<Factura> DeZona(this IQueryable<Factura> facturas, int? zonaId)  {
       return zonaId.HasValue ?                          // evalua si el parámetro zona es nulo
              facturas.Where(c => c.Zona.Id == zonaId) : // devuelve las facturas filtradas por zona
              facturas;                                  // devuelve todas las facturas
  }
}

Los métodos de extensión son estáticos y utilizan el this delante del primer parámetro. Entre otros, los suelo utilizar para encapsular filtros Where de LINQ.

Con nuestro flamante filtro de zona reutilizable podemos volver a reescribir el código inicial de la siguiente manera:

public async Task<IActionResult> OnGetAsync()
{
     Facturas = await _db.Facturas.DeZona(ZonaId).ToListAsync();
     return Page();
}

A simple vista el resultado parece mejor, ni tenemos el if en mitad de la creación de la consulta, ni necesitamos generar una variable "consulta". Visto así, parece un método correcto, pero no lo es.

En este caso nos viene bien, pero eso no significa que el método esté bien diseñado. Veámos porqué:

El parámetro que recibe el método DeZona puede ser un int o un null, por tanto el siguiente código sería perfectamente válido:

 Facturas = await _db.Facturas.DeZona(null).ToListAsync();

Y aquí viene el problema. ¿Qué devuelve el método .DeZona(null)? Obviamente, el método lo acabo de crear y sé que si le paso un null no se va a ejecutar la parte del Where. 

Pero imagina ponerte en la piel de un compañero que no ha creado el método. ¿Serías capaz de deducir qué hace el método sin necesidad de navegar a su implementación?

Podría ser que si paso un null devolviese todas las facturas cuya zona sea nula, o podría simplemente no devolver resultados porque no se permiten zonas nulas. Desde fuera no lo sé.

Aquí tienes otra versión del método con la misma firma pero diferente comportamiento:

 public static IQueryable<Factura> DeZona(this IQueryable<Factura> facturas, int? zonaId) {
      return zonaId.HasValue ?
               facturas.Where(c => c.Zona.Id == zonaId) :
               facturas.Where(c => c.Zona == null);      // devuelve las facturas cuya zona es nula
 }

Este caso representa un ejemplo de "método deshonesto", que te obliga a ir a la implementación para averiguar su comportamiento; y conviene minimizar su uso.

Una versión correcta del filtro es la siguiente:

 public static IQueryable<Factura> DeZona(this IQueryable<Factura> facturas, int zonaId /* zonaId no es nullable */) 
 {
      return facturas.Where(c => c.Zona.Id == zonaId);
 }

Con esta firma ya no hay duda de qué filtro va a aplicar.

Sin embargo, esta función nos vuelve a colocar casi como al principio, con un If en medio de las cáusulas de LINQ:

public async Task<IActionResult> OnGetAsync()
{
     var consulta = _db.Facturas.AsQueryable();

     if(ZonaId.HasValue)
          consulta = consulta.DeZona(ZonaId); // sólo hemos aplicado este cambio

      Facturas = await consulta.ToListAsync();

      return Page();
}

Es responsabilidad del "código cliente" (aquel que está usando el método) aplicar los filtros en función de los parámetros seleccionados por el usuario. Por tanto, aunque no nos guste, esta versión es más correcta.

El tema del if es otra historia y hay que atacarlo de forma individual. Para solventarlo se me ocurren dos opciones:

• Opción 1: crear otro método de extensión con un nombre diferente que describa exactamente lo que va ha hacer.

  public static IQueryable<Factura> DeZonaSiNoEsNula(this IQueryable<Factura> facturas, int? zonaId) 
  {            return zonaId.HasValue ?                       facturas.Where(c => c.Zona.Id == zonaId) :                       facturas;  }
  
  

• Opción 2: crear un método de extensión para evaluar la parte del if y que permitiese algo así:

  Facturas = await _db.Facturas                      .If(ZonaId.HasValue).Then(f=> f.DeZona(ZonaId.Value))                      .ToListAsync();   // Esta segunda opción necesitaría apoyarse en métodos If y Then como éstos:  public static (bool, IQueryable<T>) If<T>(this IQueryable<T> consulta,bool condicion)  {       return (condicion, consulta);  }   public static IQueryable<T> Then<T>(this (bool, IQueryable<T>) tupla, Func<IQueryable<T>,IQueryable<T>> funcion)  {       if (tupla.Item1)       {          return funcion(tupla.Item2);       }       return tupla.Item2;  }

   

Ambas soluciones son correctas e incluso se podrían combinar. Por ejemplo así:

// Opción 1 con métetodo con un nombre diferente
public static IQueryable<Factura> DeZonaSiNoEsNula(this IQueryable<Factura> facturas, int? zonaId)
 {
         return facturas
                    .If(zonaId.HasValue)   // Opción 2 que usa métodos de extensión 
                    .Then(facturas.DeZona(zonaId.Value));
 }

Es responsabilidad del programador aplicar en cada caso particular las herramientas adecuadas, a veces simplemente es cuestión de gustos.

.