La mayoría de las veces cuando escribo algún código que maneja la respuesta para una determinada llamada de función obtengo la siguiente estructura de código:
ejemplo: esta es una función que gestionará la autenticación para un sistema de inicio de sesión
class Authentication{
function login(){ //This function is called from my Controller
$result=$this->authenticate($username,$password);
if($result=='wrong password'){
//increase the login trials counter
//send mail to admin
//store visitor ip
}else if($result=='wrong username'){
//increase the login trials counter
//do other stuff
}else if($result=='login trials exceeded')
//do some stuff
}else if($result=='banned ip'){
//do some stuff
}else if...
function authenticate($username,$password){
//authenticate the user locally or remotely and return an error code in case a login in fails.
}
}
Problema
- Como puede ver, el código se basa en una
if/else
estructura, lo que significa que un nuevo estado de falla significará que necesito agregar unaelse if
declaración que es una violación del Principio Abierto Cerrado . - Tengo la sensación de que la función tiene diferentes capas de abstracción, ya que puedo aumentar el contador de pruebas de inicio de sesión en un controlador, pero hacer cosas más serias en otro.
- Algunas de las funciones se repiten,
increase the login trials
por ejemplo.
Pensé en convertir el múltiplo if/else
en un patrón de fábrica, pero solo usé la fábrica para crear objetos que no alteran los comportamientos. ¿Alguien tiene una mejor solución para esto?
Nota:
Este es solo un ejemplo utilizando un sistema de inicio de sesión. Estoy pidiendo una solución general a este comportamiento utilizando un patrón OO bien construido. Este tipo de if/else
controladores aparece en demasiados lugares en mi código y simplemente utilicé el sistema de inicio de sesión como un ejemplo simple y fácil de explicar. Mis casos de uso reales son demasiado complicados para publicar aquí. :RE
No limite su respuesta al código PHP y siéntase libre de usar el idioma que prefiera.
ACTUALIZAR
Otro ejemplo de código más complicado solo para aclarar mi pregunta:
public function refundAcceptedDisputes() {
$this->getRequestedEbayOrdersFromDB(); //get all disputes requested on ebay
foreach ($this->orders as $order) { /* $order is a Doctrine Entity */
try {
if ($this->isDisputeAccepted($order)) { //returns true if dispute was accepted
$order->setStatus('accepted');
$order->refund(); //refunds the order on ebay and internally in my system
$this->insertRecordInOrderHistoryTable($order,'refunded');
} else if ($this->isDisputeCancelled($order)) { //returns true if dispute was cancelled
$order->setStatus('cancelled');
$this->insertRecordInOrderHistory($order,'cancelled');
$order->rollBackRefund(); //cancels the refund on ebay and internally in my system
} else if ($this->isDisputeOlderThan7Days($order)) { //returns true if 7 days elapsed since the dispute was opened
$order->closeDispute(); //closes the dispute on ebay
$this->insertRecordInOrderHistoryTable($order,'refunded');
$order->refund(); //refunds the order on ebay and internally in my system
}
} catch (Exception $e) {
$order->setStatus('failed');
$order->setErrorMessage($e->getMessage());
$this->addLog();//log error
}
$order->setUpdatedAt(time());
$order->save();
}
}
propósito de la función:
- Estoy vendiendo juegos en eBay.
- Si un cliente desea cancelar su pedido y recuperar su dinero (es decir, un reembolso), primero debo abrir una "Disputa" en eBay.
- Una vez que se abre una disputa, debo esperar a que el cliente confirme que está de acuerdo con el reembolso (tonto, ya que él fue quien me dijo que reembolsara, pero así es como funciona en eBay).
- Esta función abre todas las disputas por mí y comprueba sus estados periódicamente para ver si el cliente ha respondido a la disputa o no.
- El cliente puede aceptar (luego reembolsar) o rechazar (luego revertir) o no responder durante 7 días (cierro la disputa yo mismo y luego devuelvo).
getOrderStrategy
es un método de fábrica que devuelve unstrategy
objeto dependiendo del estado del pedido, pero cuáles son las funcionespreProcess()
ypreProcess()
. Además, ¿por qué pasaste$this
aupdateOrderHistory($this)
?El patrón de estrategia es una buena sugerencia si realmente desea descentralizar su lógica, pero parece una exageración indirecta para ejemplos tan pequeños como los suyos. Personalmente, emplearía el patrón "escribir funciones más pequeñas", como:
fuente
Cuando comience a tener un montón de declaraciones if / then / else para manejar un estado, considere el Patrón de estado .
Hubo una pregunta sobre una forma particular de usarlo: ¿Tiene sentido esta implementación del patrón de estado?
Soy nuevo en este patrón, pero pongo la respuesta de todos modos para asegurarme de que entiendo cuándo usarlo (evite "todos los problemas se parecen a las uñas de un martillo").
fuente
Como dije en mis comentarios, la lógica compleja realmente no cambia nada.
Desea procesar un pedido en disputa. Hay múltiples formas de hacerlo. El tipo de orden en disputa puede ser
Enum
:Hay muchas maneras de hacer esto. Usted puede tener jerarquía de herencia de
Order
,DisputedOrder
,DisputedOrderLessThan7Days
,DisputedOrderCanceled
, etc. Esto no es agradable, pero también funcionaría.En mi ejemplo anterior, miro el tipo de orden y obtengo una estrategia relevante para eso. Podría encapsular ese proceso en una fábrica:
Esto vería el tipo de orden y le daría una estrategia correcta para ese tipo de orden.
Puede terminar con algo en la línea de:
Respuesta original, ya no es relevante ya que pensé que buscabas algo más simple:
Veo las siguientes preocupaciones aquí:
Yo haría lo siguiente:
Actualmente su ejemplo tiene demasiadas responsabilidades. Todo lo que hice fue encapsular esas responsabilidades dentro de los métodos. El código se ve más limpio y no tiene declaraciones de condición por todas partes.
La fábrica encapsula la construcción de objetos. No necesita encapsular la construcción de nada en su ejemplo, todo lo que necesita hacer es separar sus preocupaciones.
fuente