Entwurfsmuster für die Behandlung einer Antwort

Die meiste Zeit, wenn ich Code schreibe, der die Antwort für einen bestimmten Funktionsaufruf verarbeitet, erhalte ich die folgende Codestruktur:

Beispiel: Dies ist eine Funktion, die die Authentifizierung für ein Anmeldesystem übernimmt

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

Problem

1. Wie Sie sehen können, basiert der Code auf einer if/elseStruktur, was bedeutet, dass ein neuer Fehlerstatus bedeutet, dass ich eine else ifAnweisung hinzufügen muss , die einen Verstoß gegen das Open Closed-Prinzip darstellt .
2. Ich habe das Gefühl, dass die Funktion unterschiedliche Abstraktionsebenen aufweist, da ich möglicherweise nur den Zähler für Anmeldeversuche in einem Handler erhöhen, in einem anderen jedoch ernstere Aufgaben ausführen kann.
3. Einige der Funktionen werden beispielsweise wiederholt increase the login trials.

Ich habe darüber nachgedacht, das Multiple if/elsein ein Factory-Muster zu konvertieren , aber ich habe Factory nur verwendet, um Objekte zu erstellen, ohne das Verhalten zu ändern. Hat jemand eine bessere Lösung dafür?

Hinweis:

Dies ist nur ein Beispiel für die Verwendung eines Anmeldesystems. Ich bitte um eine allgemeine Lösung für dieses Verhalten unter Verwendung eines gut aufgebauten OO-Musters. Diese Art von if/elseHandlern erscheint an zu vielen Stellen in meinem Code und ich habe das Anmeldesystem nur als einfaches, leicht zu erklärendes Beispiel verwendet. Meine wirklichen Anwendungsfälle sind zu kompliziert, um sie hier zu posten. : D.

Bitte beschränken Sie Ihre Antwort nicht auf PHP-Code und verwenden Sie die von Ihnen bevorzugte Sprache.

AKTUALISIEREN

Ein weiteres komplizierteres Codebeispiel, um meine Frage zu klären:

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

Funktionszweck:

• Ich verkaufe Spiele bei ebay.
• Wenn ein Kunde seine Bestellung stornieren möchte und sein Geld zurückerhält (dh eine Rückerstattung), muss ich zuerst einen "Streit" bei ebay eröffnen.
• Sobald ein Streitfall eröffnet ist, muss ich warten, bis der Kunde bestätigt, dass er der Rückerstattung zustimmt (albern, da er mir gesagt hat, ich solle zurückerstatten, aber so funktioniert es bei ebay).
• Diese Funktion öffnet alle Streitigkeiten von mir und überprüft regelmäßig ihren Status, um festzustellen, ob der Kunde auf den Streit geantwortet hat oder nicht.
• Der Kunde kann zustimmen (dann erstatte ich) oder ablehnen (dann rolle ich zurück) oder 7 Tage lang nicht antworten (ich schließe den Streit selbst und erstatte dann).

Dies ist ein Hauptkandidat für das Strategiemuster .

Zum Beispiel dieser Code:

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
}

Könnte auf reduziert werden

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

Wobei getOrderStrategy die Bestellung in eine DisputeAcceptedStrategy, DisputeCancelledStrategy, DisputeOlderThan7DaysStrategy usw. einschließt, von denen jeder weiß, wie er mit der gegebenen Situation umgeht.

Bearbeiten, um Fragen in Kommentaren zu beantworten.

Könnten Sie bitte mehr über Ihren Code erfahren? Was ich verstanden habe ist, dass getOrderStrategy eine Factory-Methode ist, die abhängig vom Auftragsstatus ein Strategieobjekt zurückgibt, aber was sind die Funktionen preProcess () und preProcess (). Warum haben Sie $ this an updateOrderHistory ($ this) übergeben?

Sie konzentrieren sich auf das Beispiel, das für Ihren Fall völlig ungeeignet sein könnte. Ich habe nicht genug Details, um mir der besten Implementierung sicher zu sein, deshalb habe ich mir ein vages Beispiel ausgedacht.

Der einzige häufig verwendete Code ist insertRecordInOrderHistoryTable. Daher habe ich diesen Code (mit einem etwas allgemeineren Namen) als zentralen Punkt der Strategie verwendet. Ich übergebe $ this, weil es eine Methode mit $ order und einer anderen Zeichenfolge pro Strategie aufruft.

Im Grunde stelle ich mir jeden vor, der so aussieht:

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

Wobei $ order ein privates Mitglied der Strategie ist (denken Sie daran, ich sagte, es sollte die Reihenfolge umschließen) und das zweite Argument in jeder Klasse unterschiedlich ist. Auch dies könnte völlig unangemessen sein. Möglicherweise möchten Sie insertRecordInOrderHistoryTable in eine Basisstrategieklasse verschieben und die Authorization-Klasse nicht übergeben. Oder Sie möchten etwas ganz anderes tun, es war nur ein Beispiel.

Ebenso habe ich den Rest des unterschiedlichen Codes auf Pre- und PostProcess-Methoden beschränkt. Dies ist mit ziemlicher Sicherheit nicht das Beste, was Sie damit machen können. Geben Sie ihm passendere Namen. Teilen Sie es in mehrere Methoden auf. Was auch immer den aufrufenden Code lesbarer macht.

Sie könnte dies zu tun bevorzugen:

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

Lassen Sie einige Ihrer Strategien leere Methoden für die "IfNecessary" -Methoden implementieren.

Was auch immer den aufrufenden Code lesbarer macht.

Das Strategiemuster ist ein guter Vorschlag, wenn Sie Ihre Logik wirklich dezentralisieren möchten, aber es scheint ein Indirektions-Overkill für Beispiele zu sein, die so klein sind wie Ihre. Persönlich würde ich das Muster "kleinere Funktionen schreiben" verwenden, wie:

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

Berücksichtigen Sie das Statusmuster, wenn Sie eine Reihe von if / then / else-Anweisungen zur Behandlung eines Status haben .

Es gab eine Frage zu einer bestimmten Art der Verwendung: Ist diese Implementierung des Zustandsmusters sinnvoll?

Ich bin neu in diesem Muster, aber ich habe die Antwort trotzdem angegeben, um sicherzustellen, dass ich verstehe, wann ich sie verwenden soll (Vermeiden Sie "Alle Probleme sehen für einen Hammer wie Nägel aus").

Wie ich in meinen Kommentaren sagte, ändert komplexe Logik nichts wirklich.

Sie möchten eine umstrittene Bestellung bearbeiten. Dafür gibt es mehrere Möglichkeiten. Umstrittene Auftragsart kann sein 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();
   }
}

Es gibt viele Möglichkeiten, dies zu tun. Sie können Vererbungshierarchie von haben Order, DisputedOrder, DisputedOrderLessThan7Days, DisputedOrderCanceledetc. Das ist nicht schön, aber es würde auch funktionieren.

In meinem obigen Beispiel schaue ich mir die Auftragsart an und erhalte eine relevante Strategie dafür. Sie können diesen Prozess in eine Fabrik einkapseln:

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

Dies würde sich die Auftragsart ansehen und Ihnen eine korrekte Strategie für diese Auftragsart geben.

Sie könnten am Ende etwas in der Art von:

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

Ursprüngliche Antwort, nicht mehr relevant, da ich dachte, Sie wollten etwas Einfacheres:

Ich sehe hier folgende Bedenken:

• Überprüfen Sie, ob die IP gesperrt ist. Überprüft, ob sich die IP des Benutzers in einem gesperrten IP-Bereich befindet. Sie werden dies ausführen, egal was passiert.
• Überprüfen Sie, ob die Versuche überschritten wurden. Überprüft, ob der Benutzer seine Anmeldeversuche überschritten hat. Sie werden dies ausführen, egal was passiert.
• Benutzer authentifizieren. Versuch, den Benutzer zu authentifizieren.

Ich würde folgendes tun:

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

Derzeit hat Ihr Beispiel zu viele Verantwortlichkeiten. Alles, was ich getan habe, war, diese Verantwortlichkeiten in Methoden zusammenzufassen. Code sieht sauberer aus und Sie haben nicht überall Bedingungsanweisungen.

Factory kapselt die Konstruktion von Objekten. Sie müssen die Konstruktion von nichts in Ihrem Beispiel zusammenfassen. Alles, was Sie tun müssen, ist, Ihre Bedenken zu trennen.