Best approach - convert multiple conditional if -else in a more handy design

Question

I have a class that handles the state of a response, called StockResponse. The code has multiple ifs to handle each state of the stock. Most of the cases has a default behaviour, but some conditions need additional if-else inside to handle correctly the response. The conditions are becoming hard to maintain, and looks bad to have a huge method with many if-else conditions.

I am guessing which will be the best way to handle them. Code now looks like this:

public Result ProcessStockState(StockResponse response)
{
   if(response.state==StockStateEnum.Ok)
   {
       if(response.Sender==Sender1){ /* code....*/ }
       else if ( response.Sender==Sender2){/*code ...*/ }
       else {/**default handling code...*/}
   }
   else if(response.state==StockStateEnum.Unkown)
   {
       if(response.Sender==Sender5){ /* code....*/ }
       else if ( response.Sender==Sender7){/*code ...*/ }
       else if ( response.Sender==Sender10){/*code ...*/ }
       else {/*default handling code ...*/}
   }
   else if(response.state==X)
   {
       /*Multiple conditions that ussually differ from the other ifs*/
       /*It always have a default behaviour*/
   } 
}

Which design/pattern you would recommend me to avoid that cascading if-else ?

Answer 1

The problem with if/else chains is that they are inflexible and tightly couple that class with any implementations used if any.

For a dynamic approach, I would recommend the same solution that I mentioned in this answer.

Basically, you create a set of response handlers and a response manager. The last is in charge of dispatching the response to the proper handler.

The solution relys on polymorphism. All the handlers should implement an interface (IStockResponseHandler) with two methods:

• bool CanHandle(StockResponse response)
• Result Handle(StockResponse response)

Then, you initialize the StockResponseManager with a list of IStockResponseHandler instances.

Finally, once you get a response, you need only call the StockResponseManager.getResult(response) in order to get the proper result.

As I also mentioned in my linked answer, the benefits are future-proofing. You allow yourself to handle new types of responses with little to no additional work in your project and without adding yet another static if condition to your if/else chain. If you loaded rules from a database, you could in theory dynamically handle responses without making any changes in your program whatsoever.

As a general rule, I would create one IStockResponseHandler class per Result type. In other words, two conditions may result in the same returned Result value, and in this case it should be handled by the same class, not two separate classes. In your example I'm thinking that would be a class for each possible response.state value.

Answer 2

There is no particularly complicated pattern I would use. Just break up the top-level alternatives into individual methods like ProcessOKResponse(), ProcessUnknownResponse() etc. and leave only the dispatch logic in the entry point method.

You can later transform the big switch, e.g. into a lookup table, or into dynamic dispatch via inheritance, or something else, but the important thing is to reduce the amount of decisions taken in one block of code. Refactoring to smaller methods is the key to that.

(Example pseudo code for a method lookup table:

handlers = [
   OK: handle_ok, 
   unknown: handle_unknown, 
   backorder: handle_backorder
]

def handle_ok():
   if(response.Sender==Sender1){ /* code....*/ }
   else if ( response.Sender==Sender2){/*code ...*/ }
   else {/**default handling code...*/}

def handle_unknown():
   ...

def handle_response(response):
  handlers[response.state]()

Example code for dynamic dispatch:

class OKResponse:
    def dispatch():
        if condition1:
            action1()
        elif condition2:
            action2()
        else:
            action0()

class UnknownResponse:
    def dispatch():
        ...

def handle_response(response_object):
    response_object.dispatch()

Answer 3

Think about factoring the second-level ifs to separate functions. For example handleResponseOK() and handleResponseUnknown(). Then, your code will look like this:

public Result ProcessStockState(StockResponse response)
{
    if(response.state==StockStateEnum.Ok)
    {
        handleResponseOK(response);
    }
    else if(response.state==StockStateEnum.Unkown)
    {
        handleResponseUnknown(response);
    }
    else  if(response.state==X)
    {
        handleResponseX(response);
    }
}

When you have to make a decision, you still have to put the logic to make it somewhere and if you need more than one if... well, there's no escaping that.

Sure you can dress your code in fancy design pattern-branded clothes, but do you really need to? Maybe just good old refactoring will suffice if you just need to reduce the indentation, and don't want to code elaborate constructions to hide your ifs.

Answer 4

Considering that the main method is returning Result, you can simplify else if statements with just if + return.

And as many other guys suggested obviously it makes sense extracting if statement bodies into a method.

I guess it could look something like this:

public Result ProcessStockState(StockResponse response) {
   if(response.state==StockStateEnum.Ok) {
       return handleResponseOk();
   }

   if(response.state==StockStateEnum.Unkown) {
       return handleResponseUnknown();
   }

   if(response.state==X) {
       return handleResponseX();
   }

   return defaultResult; 
}

the same approach you can take in handleResponse methods.

Answer 5

Polymorphism is technique that can help you achieve what you need. It helps you decouple the code as well and make your code more scalable for future. Here is my implementation of the technique that breaks the if-else condition.

Create a dictionary with all the StockResponseHandlers i.e Ok,unknown,etc..

        StockEnum stockEnum = response.state;
        Dictionary<StockEnum, IStockResponseHandler> dictionary = new Dictionary<StockEnum, IStockResponseHandler>();
        dictionary.Add(StockEnum.Ok,new StockResponseOkHandler());
        dictionary.Add(StockEnum.Unknown, new StockResponseUnknownHandler());
        dictionary.Add(StockEnum.X, new StockResponderXHandler());

        if(dictionary.ContainsKey(stockEnum))
        {
            IStockResponseHandler stockResponseHandler = null;
            dictionary.TryGetValue(stockEnum,out stockResponseHandler);
            stockResponseHandler.Validate(response.Sender);
        }

Note: the last statement sends the ISender to the respective Handler

Ok Handler and other handlers would look something like this :

public class StockResponseOkHandler : IStockResponseHandler
{
    public StockResponseOkHandler()
    {
    }

    public void Validate(ISender sender)
    {
        sender.WriteSenderSpecificCode();
    }
}

You can write seperate code for each sender in their, below it is for sender 1

using System;
namespace ChainReactionWithInterface
{
    public class Sender1Handler : ISender
    {
        public Sender1Handler()
        {
        }

        public void WriteSenderSpecificCode()
        {
            Console.WriteLine("Called 1");
        }
    }
}

So we have decoupled all elements and at any point of time, we can add new conditions and it will be scalable. The only drawback i see in this technique is initialization of classes.But maybe there are other of optimizing it for good by lazy loading or some sorta thing.

Answer 6

Using Java enum pseudocode:

public enum StockStateEnum {

    OK(
        public void process(Response response) {
          if(response.Sender==Sender1){ /* code....*/ }
          else if ( response.Sender==Sender2){/*code ...*/ }
          else {/**default handling code...*/}
        }
    ),

    UNKOWN(
      public void process(Response response) { 
        if(response.Sender==Sender5){ /* code....*/ }
        else if ( response.Sender==Sender7){/*code ...*/ }
        else if ( response.Sender==Sender10){/*code ...*/ }
        else {/*default handling code ...*/}
      }
    ),
    ...

    public abstract process(Response response);

    public static StockStateEnum findByState(Response response) {
      // Loop through enum values trying to find a match
      for (StockStateEnum stockState : StockStateEnum.values() {
        if (stockState == response.state) {
          return stockState;
        }
      }
      return StockStateEnum.DEFAULT;  // Or null, depending on requirement
    }
}

Usage:

public Result ProcessStockState(StockResponse response) {
  StockStateEnum stockState = StockStateEnum.findByState(response);
  stockState.process(response);
  ...
}

My guess is you could do something similar with response.Sender reducing even more the number of if statements and cyclomatic complexity.