Wzorzec projektowy do obsługi odpowiedzi

Przez większość czasu, gdy piszę kod, który obsługuje odpowiedź na określone wywołanie funkcji, otrzymuję następującą strukturę kodu:

przykład: jest to funkcja, która będzie obsługiwać uwierzytelnianie w systemie logowania

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. Jak widać, kod jest zbudowany na if/elsestrukturze, co oznacza, że ​​nowy stan awarii oznacza, że ​​muszę dodać else ifoświadczenie, które narusza zasadę otwartej zamkniętej zasady .
2. Mam wrażenie, że funkcja ma różne warstwy abstrakcji, ponieważ mogę po prostu zwiększyć licznik prób logowania w jednym module obsługi, ale zrobić coś poważniejszego w innym.
3. Niektóre funkcje są increase the login trialsna przykład powtarzane .

Myślałem o przekształceniu wielokrotności if/elsena wzorzec fabryczny, ale użyłem fabryki tylko do tworzenia obiektów, a nie zmiany zachowań. Czy ktoś ma na to lepsze rozwiązanie?

Uwaga:

To tylko przykład z wykorzystaniem systemu logowania. Proszę o ogólne rozwiązanie tego zachowania przy użyciu dobrze zbudowanego wzorca OO. Tego rodzaju if/elseprogramy obsługi pojawiają się w zbyt wielu miejscach w moim kodzie i właśnie użyłem systemu logowania jako prostego, łatwego do wyjaśnienia przykładu. Moje prawdziwe przypadki użycia są zbyt skomplikowane, aby opublikować tutaj. :RE

Nie ograniczaj swojej odpowiedzi do kodu PHP i skorzystaj z preferowanego języka.

AKTUALIZACJA

Kolejny bardziej skomplikowany przykład kodu, aby wyjaśnić moje pytanie:

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

cel funkcji:

• Sprzedaję gry w serwisie eBay.
• Jeśli klienci chcą anulować swoje zamówienie i odzyskać pieniądze (tj. Zwrot pieniędzy), najpierw muszę otworzyć „spór” w serwisie eBay.
• Gdy spór zostanie otwarty, muszę poczekać, aż klient potwierdzi, że zgadza się na zwrot pieniędzy (głupie, ponieważ to on kazał mi zwrócić pieniądze, ale tak to działa w serwisie eBay).
• Ta funkcja otwiera wszystkie spory przeze mnie i okresowo sprawdza ich status, aby sprawdzić, czy klient odpowiedział na spór, czy nie.
• Klient może wyrazić zgodę (następnie zwracam pieniądze) lub odmówić (następnie wycofuję) lub może nie odpowiadać przez 7 dni (sam zamykam spór, a następnie zwracam pieniądze).

Jest to główny kandydat do wzorca strategii .

Na przykład ten kod:

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
}

Można zmniejszyć do

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

gdzie getOrderStrategy pakuje zamówienie w DisputeAcceptedStrategy, DisputeCancelledStrategy, DisputeOlderThan7DaysStrategy itp., z których każdy wie, jak poradzić sobie w danej sytuacji.

Edytuj, aby odpowiedzieć na pytanie w komentarzach.

czy mógłbyś bardziej rozwinąć swój kod? Zrozumiałem, że getOrderStrategy to metoda fabryczna, która zwraca obiekt strategii w zależności od statusu zamówienia, ale jakie są funkcje preProcess () i preProcess (). Także dlaczego przekazałeś $ this, aby zaktualizowaćOrderHistory ($ this)?

Skupiasz się na przykładzie, który może być całkowicie nieodpowiedni dla twojej sprawy. Nie mam wystarczająco dużo szczegółów, aby mieć pewność najlepszej realizacji, więc wpadłem na niejasny przykład.

Jednym z powszechnych elementów kodu jest insertRecordInOrderHistoryTable, dlatego postanowiłem użyć go (z nieco bardziej ogólną nazwą) jako centralnego punktu strategii. Przekazuję to $ $, ponieważ wywołuje to metodę z $ order i innym łańcuchem na strategię.

Zasadniczo wyobrażam sobie każdego z tych, którzy wyglądają tak:

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

Gdzie $ order jest prywatnym członkiem Strategii (pamiętaj, że powiedziałem, że powinno ono zawinąć zamówienie), a drugi argument jest inny w każdej klasie. Ponownie może to być całkowicie niewłaściwe. Możesz przenieść insertRecordInOrderHistoryTable do podstawowej klasy strategii i nie przekazywać klasy autoryzacji. Możesz też zrobić coś zupełnie innego, to był tylko przykład.

Podobnie, resztę różniącego się kodu ograniczyłem do metod poprzedzających i następujących po przetwarzaniu. To prawie na pewno nie najlepsze, co możesz z tym zrobić. Nadaj mu bardziej odpowiednie nazwy. Podziel go na wiele metod. Cokolwiek sprawia, że ​​kod wywołujący jest bardziej czytelny.

Możesz to zrobić:

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

I niech niektóre z twoich strategii wdrożą puste metody dla metod „IfNecessary”.

Cokolwiek sprawia, że ​​kod wywołujący jest bardziej czytelny.

Wzorzec strategii jest dobrą sugestią, jeśli naprawdę chcesz zdecentralizować swoją logikę, ale wydaje się, że przesada pośrednia dla przykładów tak małych jak twoje. Osobiście zastosowałbym wzorzec „pisz mniejsze funkcje”, na przykład:

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

Kiedy zaczniesz mieć kilka instrukcji if / then / else do obsługi statusu, rozważ wzorzec stanu .

Pojawiło się pytanie o konkretny sposób jego wykorzystania: Czy ta implementacja wzorca stanu ma sens?

Jestem nowy w tym tupocie, ale i tak postawiłem odpowiedź, aby upewnić się, że rozumiem, kiedy jej użyć (Unikaj „wszystkie problemy wyglądają jak gwoździe do młota”).

Jak powiedziałem w moich komentarzach, złożona logika tak naprawdę niczego nie zmienia.

Chcesz zrealizować sporne zamówienie. Można to zrobić na wiele sposobów. Spornym rodzajem zamówienia może być 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();
   }
}

Można to zrobić na wiele sposobów. Można mieć hierarchię dziedziczenia Order, DisputedOrder, DisputedOrderLessThan7Days, DisputedOrderCanceled, itd. Nie jest to miłe, ale również praca.

W powyższym przykładzie patrzę na rodzaj zamówienia i szukam odpowiedniej strategii. Możesz zamknąć ten proces w fabryce:

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

Spowoduje to sprawdzenie rodzaju zamówienia i zapewni właściwą strategię dla tego rodzaju zamówienia.

Możesz skończyć z czymś w rodzaju:

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

Oryginalna odpowiedź, nieistotna, ponieważ myślałem, że szukasz czegoś prostszego:

Widzę tutaj następujące obawy:

• Sprawdź zablokowane IP. Sprawdza, czy adres IP użytkownika znajduje się w zabronionym zakresie adresów IP. Wykonasz to bez względu na wszystko.
• Sprawdź, czy próby zostały przekroczone. Sprawdza, czy użytkownik przekroczył liczbę prób logowania. Wykonasz to bez względu na wszystko.
• Uwierzytelnij użytkownika. Próba uwierzytelnienia użytkownika.

Zrobiłbym następujące:

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

Obecnie twój przykład ma zbyt wiele obowiązków. Wszystko, co zrobiłem, to ujęcie tych obowiązków w ramach metod. Kod wygląda na bardziej przejrzysty i nie ma wszędzie instrukcji warunkowych.

Fabryka obudowuje konstrukcję obiektów. Nie musisz ujmować konstrukcji niczego w swoim przykładzie, wszystko, co musisz zrobić, to oddzielić swoje obawy.