Patrón de diseño para manejar una respuesta

10

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

  1. Como puede ver, el código se basa en una if/elseestructura, lo que significa que un nuevo estado de falla significará que necesito agregar una else ifdeclaración que es una violación del Principio Abierto Cerrado .
  2. 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.
  3. Algunas de las funciones se repiten, increase the login trialspor ejemplo.

Pensé en convertir el múltiplo if/elseen 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/elsecontroladores 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).
Songo
fuente

Respuestas:

15

Este es un candidato principal para el patrón de estrategia .

Por ejemplo, este código:

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
}

Podría reducirse a

var $strategy = $this.getOrderStrategy($order);
$strategy->preProcess();
$strategy->updateOrderHistory($this);
$strategy->postProcess();

donde getOrderStrategy envuelve el pedido en una DisputeAcceptedStrategy, DisputeCancelledStrategy, DisputeOlderThan7DaysStrategy, etc., cada uno de los cuales sabe cómo manejar la situación dada.

Editar, para responder preguntas en los comentarios.

¿podría por favor elaborar más sobre su código? Lo que entendí es que getOrderStrategy es un método de fábrica que devuelve un objeto de estrategia dependiendo del estado del pedido, pero cuáles son las funciones preProcess () y preProcess (). Además, ¿por qué pasó $ this a updateOrderHistory ($ this)?

Te estás centrando en el ejemplo, que podría ser completamente inapropiado para tu caso. No tengo suficientes detalles para estar seguro de la mejor implementación, así que se me ocurrió un ejemplo vago.

El único código común que tiene es insertRecordInOrderHistoryTable, por lo que elegí usar eso (con un nombre un poco más genérico) como punto central de la estrategia. Le paso $ this, porque está llamando a un método en esto, con $ order y una cadena diferente por estrategia.

Entonces, básicamente, imagino a cada uno de los que se ven así:

public function updateOrderHistory($auth) {
    $auth.insertRecordInOrderHistoryTable($order, 'cancelled');
}

Donde $ order es un miembro privado de la Estrategia (recuerde que dije que debería envolver el pedido) y el segundo argumento es diferente en cada clase. De nuevo, esto podría ser completamente inapropiado. Es posible que desee mover insertRecordInOrderHistoryTable a una clase de estrategia base y no pasar la clase de autorización. O puede que desee hacer algo completamente diferente, era solo un ejemplo.

Del mismo modo, limité el resto del código diferente a los métodos pre y postProcess. Es casi seguro que no es lo mejor que puedes hacer con él. Dale nombres más apropiados. Dividirlo en múltiples métodos. Lo que sea que haga que el código de llamada sea más legible.

Es posible que prefiera hacer esto:

var $strategy = $this.getOrderStrategy($order);
$strategy->setStatus();
$strategy->closeDisputeIfNecessary();
$strategy->refundIfNecessary();
$strategy->insertRecordInOrderHistoryTable($this);                        
$strategy->rollBackRefundIfNecessary();

Y haga que algunas de sus Estrategias implementen métodos vacíos para los métodos "Si es necesario".

Lo que sea que haga que el código de llamada sea más legible.

pdr
fuente
Gracias por su respuesta, pero ¿podría dar más detalles sobre su código? Lo que entendí es que getOrderStrategyes un método de fábrica que devuelve un strategyobjeto dependiendo del estado del pedido, pero cuáles son las funciones preProcess()y preProcess(). Además, ¿por qué pasaste $thisa updateOrderHistory($this)?
Songo
1
@Songo: Espero que la edición anterior ayude.
pdr
¡Ajá! Creo que lo entiendo ahora. Definitivamente un voto positivo de mi parte :)
Songo
+1, Puede, usted elabora, si la línea, var $ estrategia = $ this.getOrderStrategy ($ orden); tendría un caso de cambio para identificar la estrategia.
Naveen Kumar
2

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:

if($result=='wrong password')
   wrongPassword();
else if($result=='wrong username')
   wrongUsername();
else if($result=='login trials exceeded')
   excessiveTries();
else if($result=='banned ip')
   bannedIp();
Karl Bielefeldt
fuente
1

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").

JeffO
fuente
0

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:

public void ProcessDisputedOrder(DisputedOrder order)
{
   switch (order.Type)
   {
       case DisputedOrderType.Canceled:
          var strategy = new StrategyForDisputedCanceledOrder();
          strategy.Process(order);  
          break;

       case DisputedOrderType.LessThan7Days:
          var strategy = new DifferentStrategy();
          strategy.Process(order);
          break;

       default: 
          throw new NotImplementedException();
   }
}

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:

var strategy = DisputedOrderStrategyFactory.Instance.Build(order.Type);

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:

public void ProcessDisputedOrder(DisputedOrder order)
{
   var strategy = DisputedOrderStrategyFactory.Instance.Build(order.Type);   
   strategy.Process(order);
}

Respuesta original, ya no es relevante ya que pensé que buscabas algo más simple:

Veo las siguientes preocupaciones aquí:

  • Verifique la IP prohibida. Comprueba si la IP del usuario está en un rango de IP prohibido. Ejecutarás esto sin importar qué.
  • Verificar si se superaron las pruebas. Comprueba si el usuario ha excedido sus intentos de inicio de sesión. Ejecutarás esto sin importar qué.
  • Autenticar usuario. Intenta autenticar al usuario.

Yo haría lo siguiente:

CheckBannedIP(login.IP);
CheckLoginTrial(login);

Authenticate(login.Username, login.Password);

public void CheckBannedIP(string ip)
{
    // If banned then re-direct, else do nothing.
}

public void CheckLoginTrial(LoginAttempt login)
{
    // If exceeded trials, then inform user, else do nothing
}

public void Authenticate(string username, string password)
{
     // Attempt to authenticate. On success redirect, else catch any errors and inform the user. 
}

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.

CodeART
fuente
Gracias por su respuesta, pero mis controladores para cada estado de respuesta pueden ser realmente complejos. por favor vea la actualización de la pregunta.
Songo
No cambia nada. La responsabilidad es procesar el orden en disputa utilizando alguna estrategia. La estrategia variaría según el tipo de disputa.
CodeART
Por favor vea una actualización. Para una lógica más compleja, puede utilizar la fábrica para construir sus estrategias de pedido en disputa.
CodeART
1
+1 Gracias por la actualización. Ahora es mucho más claro.
Songo