Hace poco, trabajando en una mejora para un sistema que desarrollé para un cliente me pasó lo siguiente:
Una parte del trabajo de la aplicación era obtener información financiera de diferentes fuentes, básicamente se trataba de obtener precios históricos de bonos.
Existían diferentes fuentes de consulta debido a que la información no siempre estaba disponible en todos los sitios (más allá de no disponer de APIs, pero esa es otra historia).
El punto es que, en la primera versión de la aplicación, que obviamente estaba desarrollada sobre el framework Symfony, simplemente creamos un método dentro del Controlador:
private function fetchBondPrice($symbol, \DateTime $date)
{
try {
if ($price = $this->fetchBondPriceFromQuoteNet($symbol, $date)) {
return $price;
}
if ($price = $this->fetchBondPriceFromMorningStar($symbol, $date)) {
return $price;
}
} catch (Exception $e) {
}
return null;
}
Y las cosas funcionaron… pero desde el comienzo quedó pendiente este comentario en el código:
* @todo Refactor into a Strategy Pattern
El momento oportuno llegó cuando debimos alterar algunos aspectos de las búsquedas específicas y, para asegurarnos de que todo saliera bien, decidimos usar PHPUnit y ahí se hizo casi obvia la necesidad de refactorizar el código.
La primera vuelta del refactor nos trajo consigo una clase sencilla, un Service de Symfony llamado BondPriceFetcherManager.
Que, a priori no era mucho más que una forma de deslindar responsabilidad desde el controlador hacia una clase más específica… pero seguíamos teniendo un método que había que modificar si queríamos incorporar una nueva fuente de datos (Algo que muy probablemente se venía).
Por otro lado, fue evidente que algo andaba mal cuando tuvimos que cambiar los métodos fetchBondPriceFromQuoteNet y fetchBondPriceFromMorningStar de privados a públicos para poder hacer los tests…Había llegado el momento de implementar el patrón Strategy.
Muy brevemente, lo que hicimos fue crear una clase para cada fuente de datos: QuoteNetBondPriceFetcher y MorningStarBondPriceFetcher.
Ambas derivadas del nuevo BondPriceFetcher que simplemente se ve así:
<?php
abstract class BondPriceFetcher
{
/**
* @param $symbol
* @param \DateTime $date
* @return mixed
*/
abstract public function fetchPrice($symbol, \DateTime $date);
}
Ahora sí las cosas se ponen interesantes 🙂
Después queda una nueva clase genérica BondPriceFetcherManager que tiene un método:
public function fetchBondPrice($symbol, \DateTime $date)
{
$price = null;
foreach ($this->getFetchers() as $fetcher ) {
try {
if ( $price = $fetcher->fetchPrice( $symbol, $date) ) {
break;
}
} catch ( Exception $e ) {
$this->logger->addDebug( 'Error buscando precio del bono: '.$symbol.'-'.$date->format('Y-m-d').' usando '.get_class($fetcher).': '.$e->getMessage() );
}
}
return $price;
}
Obviamente, también tiene su método
public function addFetcher( BondPriceFetcher $fetcher )
{
$this->fetchers[] = $fetcher;
return $this;
}
Y
/**
* @param Logger $logger
* @return BondPriceFetcherManager
*/
public function setLogger(Logger $logger)
{
$this->logger = $logger;
return $this;
}
Y listo! Ya nos quedó una muy linda implementación que permite:
- Extender la cantidad de fuentes de datos (Basta con crear un nuevo
BondPriceFetchery agregar una nueva llamada aaddFetcher) - Alterar la prioridad que se le da a cada fuente (Basta con cambiar el orden de las llamadas a
addFetcher) y sin alterar en absoluto el código propio delFetcherManager - Testear automáticamente toda esta funcionalidad
- Reutilizar esta funcionalidad (Si se hiciera en un Bundle por ejemplo)
Debo reconocer que, si bien no soy un gran fanático de los patrones de diseño, Strategy es definitivamente mi favorito 🙂
El mayor aporte del patrón Strategy es el desacoplamiento (además de generar un código más elegante a fuerza de un poco más de texto)
¿Cómo fue tu experiencia con la aplicación de patrones de diseño?
- Cómo enviarencabezados SOAP desde PHP - 09/12/2024
- Por qué PHP 8 no satisface el requisito ^7.3 de composer - 09/12/2024
- Cómo usar PHPUnit - 03/12/2024
Algunos apuntes:
Si bien el patrón Strategy no está pensado para albergar más de una estrategia al mismo tiempo (la gracia de este patrón es poder configurar la estrategia en runtime), la implementación que comentas es bastante habitual y yo mismo la he usado en algunas ocasiones por lo potente que es.
Creo que hay algunos puntos que, desde mi punto de vista, podrían mejorar la implementación:
BondPriceFetcher debería de ser una interfaz, no hay ninguna implementación heredada, así que no requiere ser una clase abstracta
BondPriceFetcherManager sufre de acoplamiento temporal: hasta que no se hayan hecho llamadas a addFetcher no sirve para nada. Es una buena idea pasar por constructor los fetchers necesarios: __construct(BondPriceFetcher …$fetchers)
Tanto si aplicamos el anterior punto como si no, la construcción del servicio puede ser problemática (por acoplamiento temporal o por repetir la instanciación de fetchers en determinado orden en el constructor. Crear una factory class con métodos estáticos para crear instancias del manager con las configuraciones más habituales podría ser interesante.
El manejo de la prioridad de cada fetcher es demasiado precario, en caso de querer alterar el orden deberíamos de reconstruir el objeto de nuevo. BondPriceFetcher debería de definir un
getPriority()y cada fetcher recibir en su constructor la prioridad, de esta forma el orden del código no afectaría de forma directa a su funcionamiento.Saludos!
Hola Miguel!
Gracias por tus aportes! Muy interesantes 🙂
En el caso de este código, la idea era poder combinar diferentes estrategias a modo de fallback más que decidir en forma dinámica cuál usar en cada momento.
Sobre el tema que comentas del BondPriceFetcher, puede que en el post no haya quedado claro, pero en el sistema real existen un par de subclases. Aunque no estaría muy convencido de que fuese esa la razón para hacerlo una interfaz, en principio porque tiene algunos métodos definidos que podrían servir a futuras subclases.
Muy interesante lo del acoplamiento temporal, no lo había escuchado antes y sí, tienes razón, sería mejor que el propio constructor recibiera los BondFetchers porque, como bien señalas, sin eso no tiene utilidad alguna.
Sobre el tema de la prioridad también coincido, sería mejor desacoplarlo del orden de instanciación. En este caso particular no fue realmente necesario hacerlo pero sí, definitivamente sería lo más correcto.
Saludos!
Hola!
Respecto a lo de la interface, me refiero a que actualmente BondPriceFetcher es así:
abstract class BondPriceFetcher
{
abstract public function fetchPrice($symbol, \DateTime $date);
}
Una clase abstracta sin comportamiento propio, solamente define un método que hay deben de implementar QuoteNetBondPriceFetcher y MorningStarBondPriceFetcher, por lo que podrías convertir BondPriceFetcher en una interface:
interface BondPriceFetcher
{
public function fetchPrice($symbol, \DateTime $date);
}
Que QuoteNetBondPriceFetcher y MorningStarBondPriceFetcher implementen. De esa manera te aseguras que cumplen el contrato necesario para poder ser fetchers pero sin usar herencia, solo composición.
Si por ejemplo hicieras el tema de la prioridad que comentábamos antes, podría tener más sentido que fuese una clase abstracta:
abstract class BondPriceFetcher
{
private int $priority;
public function __construct(int $priority = 0){
$this->priority = $priority;
}
public function getPriority(): int
{
return $this->priority;
}
abstract public function fetchPrice($symbol, \DateTime $date);
}
De manera que podrías heredar la funcionalidad de la prioridad en cada uno de los fetchers sin necesidad de duplicarla.
Saludos!
Hola!
Pues sí, coincido con tus comentarios. El post ha quedado algo desfasado respecto de la implementación actual (que también se merecería un pequeño refactor :p).
Muy buenos aportes!