Padrão de design para manipular uma resposta

10

Na maioria das vezes, quando estou escrevendo algum código que lida com a resposta para uma determinada chamada de função, recebo a seguinte estrutura de código:

exemplo: Esta é uma função que manipulará a autenticação para um sistema de login

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 você pode ver, o código é construído em uma if/elseestrutura, o que significa que um novo status de falha significa que preciso adicionar uma else ifdeclaração que seja uma violação do Princípio Aberto Fechado .
  2. Sinto que a função tem diferentes camadas de abstração, pois posso apenas aumentar o contador de tentativas de login em um manipulador, mas fazer coisas mais sérias em outro.
  3. Algumas das funções são repetidas, increase the login trialspor exemplo.

Pensei em converter o múltiplo if/elseem um padrão de fábrica, mas só usei o factory para criar objetos e não alterar comportamentos. Alguém tem uma solução melhor para isso?

Nota:

Este é apenas um exemplo usando um sistema de login. Estou pedindo uma solução geral para esse comportamento usando um padrão OO bem construído. Esse tipo de if/elsemanipulador aparece em muitos lugares no meu código e eu apenas usei o sistema de login como um exemplo simples e fácil de explicar. Meus casos de uso reais são muito complicados para postar aqui. : D

Por favor, não limite sua resposta ao código PHP e sinta-se à vontade para usar a linguagem de sua preferência.


ATUALIZAR

Outro exemplo de código mais complicado apenas para esclarecer minha pergunta:

  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();
        }
    }

finalidade da função:

  • Eu estou vendendo jogos no ebay.
  • Se um cliente deseja cancelar seu pedido e receber seu dinheiro de volta (ou seja, um reembolso), primeiro devo abrir uma "disputa" no ebay.
  • Depois que uma disputa é aberta, devo esperar que o cliente confirme que ele concorda com o reembolso (por mais tolo que ele tenha me dito para reembolsar, mas é assim que funciona no ebay).
  • Essa função obtém todas as disputas abertas por mim e verifica seus status periodicamente para ver se o cliente respondeu ou não à disputa.
  • O cliente pode concordar (depois reembolso) ou recusar (depois reverter) ou pode não responder por 7 dias (eu mesmo encerro a disputa e reembolso).
Songo
fonte

Respostas:

15

Este é um candidato principal para o padrão de estratégia .

Por exemplo, 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
}

Pode ser reduzido para

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

onde getOrderStrategy agrupa o pedido em DisputeAcceptedStrategy, DisputeCancelledStrategy, DisputeOlderThan7DaysStrategy, etc., cada um dos quais sabe como lidar com a situação especificada.

Edite para responder a pergunta nos comentários.

você poderia elaborar mais sobre seu código. O que eu entendi é que getOrderStrategy é um método de fábrica que retorna um objeto de estratégia dependendo do status do pedido, mas quais são as funções preProcess () e preProcess (). Além disso, por que você passou $ this para updateOrderHistory ($ this)?

Você está se concentrando no exemplo, que pode ser completamente inapropriado para o seu caso. Como não tenho detalhes suficientes para garantir a melhor implementação, criei um exemplo vago.

O único pedaço de código comum que você tem é insertRecordInOrderHistoryTable, então eu escolhi usá-lo (com um nome um pouco mais genérico) como ponto central da estratégia. Eu passo $ this para ele, porque está chamando um método para isso, com $ order e uma string diferente por estratégia.

Então, basicamente, eu imagino cada um parecido com isto:

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

Onde $ order é um membro privado da Estratégia (lembre-se de que eu disse que deve agrupar a ordem) e o segundo argumento é diferente em cada classe. Novamente, isso pode ser totalmente inapropriado. Você pode mover o insertRecordInOrderHistoryTable para uma classe Strategy básica e não passar a classe Authorization. Ou pode fazer algo completamente diferente, foi apenas um exemplo.

Da mesma forma, limitei o restante do código diferente aos métodos pré e pós-processo. Isso quase certamente não é o melhor que você pode fazer com isso. Dê nomes mais apropriados. Divida-o em vários métodos. O que quer que torne o código de chamada mais legível.

Você pode preferir fazer isso:

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

E faça com que algumas de suas estratégias implementem métodos vazios para os métodos "IfNecessary".

O que quer que torne o código de chamada mais legível.

pdr
fonte
Obrigado pela sua resposta, mas você poderia elaborar mais no seu código. O que eu entendi é que getOrderStrategyé um método de fábrica que retorna um strategyobjeto dependendo do status do pedido, mas quais são as funções preProcess()e preProcess(). Além disso, por que você passar $thispara updateOrderHistory($this)?
Songo
11
@Ongo: Espero que a edição acima ajude.
PDR
Aha! Eu acho que entendi agora. Definitivamente um up-voto de mim :)
Songo
+1, você pode especificar se a linha var $ strategy = $ this.getOrderStrategy ($ order); teria um caso de mudança para identificar a estratégia.
Naveen Kumar
2

O padrão de estratégia é uma boa sugestão se você realmente deseja descentralizar sua lógica, mas parece um exagero indireto para exemplos tão pequenos quanto o seu. Pessoalmente, eu empregaria o padrão "escrever funções menores", 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
fonte
1

Quando você começar a ter várias instruções if / then / else para manipular um status, considere o Padrão de Estado .

Havia uma pergunta sobre uma maneira específica de usá-lo: essa implementação do padrão de estado faz sentido?

Eu sou novo nesse padrão, mas, de qualquer maneira, respondo a resposta para ter certeza de que entenderá quando usá-lo (evite "todos os problemas parecem unhas de um martelo").

JeffO
fonte
0

Como eu disse nos meus comentários, a lógica complexa realmente não muda nada.

Você deseja processar um pedido contestado. Existem várias maneiras de fazer isso. O tipo de pedido contestado pode 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();
   }
}

Existem muitas maneiras de fazer isso. Você pode ter hierarquia de herança Order, DisputedOrder, DisputedOrderLessThan7Days, DisputedOrderCanceled, etc. Isto não é bom, mas também funcionaria.

No meu exemplo acima, analiso o tipo de pedido e obtenho uma estratégia relevante para isso. Você pode encapsular esse processo em uma fábrica:

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

Isso analisaria o tipo de pedido e forneceria uma estratégia correta para esse tipo de pedido.

Você pode acabar com algo nas linhas de:

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

Resposta original, não é mais relevante como eu pensava que você estava procurando por algo mais simples:

Vejo as seguintes preocupações aqui:

  • Verifique se há IP banido. Verifica se o IP do usuário está em um intervalo de IP banido. Você executará isso não importa o quê.
  • Verifique se as tentativas foram excedidas. Verifica se o usuário excedeu suas tentativas de login. Você executará isso não importa o quê.
  • Autenticar usuário. Tentativa de autenticar usuário.

Eu faria o seguinte:

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. 
}

Atualmente, seu exemplo tem muitas responsabilidades. Tudo o que fiz foi encapsular essas responsabilidades dentro dos métodos. O código parece mais limpo e você não tem instruções de condição em todo o lugar.

A fábrica encapsula a construção de objetos. Você não precisa encapsular a construção de nada no seu exemplo, tudo o que você precisa fazer é separar suas preocupações.

CodeART
fonte
Obrigado pela sua resposta, mas meus manipuladores para cada status de resposta podem ser realmente complexos. por favor, veja a atualização da pergunta.
Songo
Isso não muda nada. A responsabilidade é processar o pedido em disputa usando alguma estratégia. A estratégia varia de acordo com o tipo de disputa.
CodeART
Por favor, veja uma atualização. Para uma lógica mais complexa, você pode utilizar o factory para criar suas estratégias de pedidos em disputa.
CodeART
11
+1 Obrigado pela atualização. É muito mais claro agora.
Songo