Question

Most of the time when I'm writing some code that handles the response for a certain function call I get the following code structure:

example: This is a function that will handle the authentication for a login system

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. As you can see the code is build on a if/else structure which means a new failure status will mean that I need to add an else if statement which is a violation for the Open Closed Principle.
  2. I get a feeling that the function has different layers of abstraction as I may just increase the login trials counter in one handler, but do more serious stuff in another.
  3. Some of the functions are repeated increase the login trials for example.

I thought about converting the multiple if/else to a factory pattern, but I only used factory to create objects not alter behaviors. Does anyone have a better solution for this?

Note:

This is just an example using a login system. I'm asking for a general solution to this behavior using a well built OO pattern. This kind of if/else handlers appears in too many places in my code and I just used the login system as a simple easy to explain example. My real use cases are to much complicated to post here. :D

Please don't limit your answer to PHP code and feel free to use the language you prefer.


UPDATE

Another more complicated code example just to clarify my question:

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

function purpose:

  • I am selling games on ebay.
  • If a customers wishes to cancel his order and gets his money back (i.e. a Refund) I must open a "Dispute" on ebay first.
  • Once a dispute is opened I must wait for the customer to confirm that he agrees to the refund (silly as he's the one who told me to refund, but that's how it works on ebay).
  • This functions gets all disputes opened by me and checks their statuses periodically to see if the customer has replied to the dispute or not.
  • The customer may agree (then I refund) or refuse (then I rollback) or may not respond for 7 days (I close the dispute myself then refund).

No correct solution

Licensed under: CC-BY-SA with attribution
scroll top