Design pattern per la gestione di una risposta

Questo è un candidato ideale per il modello di strategia .

Ad esempio, questo codice:

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
}

Potrebbe essere ridotto a

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

dove getOrderStrategy esegue il wrapping dell'ordine in una DisputeAcceptedStrategy, DisputeCancelledStrategy, DisputeOlderThan7DaysStrategy, ecc. ognuno dei quali sa come gestire la situazione data.

Modifica, per rispondere alla domanda nei commenti.

could you please elaborate more on your code. What I understood is that getOrderStrategy is a factory method that returns a strategy object depending on the order status, but what are the preProcess() and preProcess() functions. Also why did you pass $this to updateOrderHistory($this)?

Ti stai concentrando sull'esempio, che potrebbe essere completamente inappropriato per il tuo caso. Non ho abbastanza dettagli per essere sicuro della migliore implementazione, quindi ho trovato un esempio vago.

L'unico codice comune che hai è insertRecordInOrderHistoryTable, quindi ho scelto di usarlo (con un nome leggermente più generico) come punto centrale della strategia. Ho passato $ a questo, perché sta chiamando un metodo su questo, con ordine $ e una stringa diversa per strategia.

Quindi, in sostanza, immagino che ognuno di questi abbia questo aspetto:

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

Dove $ order è un membro privato della strategia (ricorda che ho detto che dovrebbe avvolgere l'ordine) e il secondo argomento è diverso in ogni classe. Ancora una volta, questo potrebbe essere del tutto inappropriato. Potresti voler spostare insertRecordInOrderHistoryTable in una classe Strategy di base e non passare la classe Authorization. O potresti voler fare qualcosa di completamente diverso, era solo un esempio.

Allo stesso modo, ho limitato il resto del codice diverso ai metodi pre- e postProcess. Questo è quasi certamente il meglio che puoi fare con esso. Dagli nomi più appropriati. Suddividilo in più modi. Qualunque cosa renda il codice chiamante più leggibile.

potresti preferire questo:

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

E alcune delle tue strategie implementano metodi vuoti per i metodi "IfNecessari".

Qualunque cosa rende il codice chiamante più leggibile.

Il modello di strategia è un buon suggerimento se vuoi veramente decentralizzare la tua logica, ma sembra un eccesso eccessivo di indiretti per esempi piccoli come i tuoi. Personalmente, utilizzerei il modello "scrivi funzioni più piccole", come:

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

Quando inizi a disporre di una serie di istruzioni if / then / else per gestire uno stato, considera il Pattern di stato .

C'era una domanda su un particolare modo di usarlo: Questa implementazione dello schema di stato ha senso?

Sono nuovo di questo patter, ma ho comunque dato la risposta per essere sicuro di capire quando usarlo (Evita "tutti i problemi sembrano chiodi su un martello.").

Come ho detto nei miei commenti, la logica complessa in realtà non cambia nulla.

Vuoi elaborare un ordine contestato. Ci sono molti modi per farlo. Il tipo di ordine contestato può essere 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();
   }
}

Ci sono molti modi per farlo. Puoi avere una gerarchia di ereditarietà di Order , DisputedOrder , DisputedOrderLessThan7Days , DisputedOrderCanceled , ecc. Questo non è bello, ma funzionerebbe anche.

Nel mio esempio sopra, guardo il tipo di ordine e ottengo una strategia pertinente per questo. Potresti incapsulare quel processo in una fabbrica:

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

Verrebbe esaminato il tipo di ordine e ti daremo una strategia corretta per quel tipo di ordine.

Potresti finire con qualcosa sulle linee di:

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

Risposta originale, non più pertinente poiché pensavo che fossi alla ricerca di qualcosa di più semplice:

Vedo le seguenti preoccupazioni qui:

• Verifica l'IP vietato. Controlla se l'IP dell'utente si trova in un intervallo IP vietato. Lo eseguirai indipendentemente da cosa.
• Verifica se le prove hanno superato. Controlla se l'utente ha superato i propri tentativi di accesso. Lo eseguirai indipendentemente da cosa.
• Autentica utente. Tenta di autenticare l'utente.

Farei quanto segue:

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

Attualmente il tuo esempio ha troppe responsabilità. Tutto ciò che ho fatto, è stato incapsulare quelle responsabilità all'interno dei metodi. Il codice sembra più pulito e non si hanno dichiarazioni condizionali dappertutto.

La fabbrica incapsula la costruzione di oggetti. Non è necessario incapsulare la costruzione di qualcosa nel tuo esempio, tutto ciò che devi fare è separare le tue preoccupazioni.