Big if else statement

Question

If have a big ( about 100 plus) if else statement like below and the if else condition might be irregular(for example some depends on 3 variables, some on 4), is there any way of making it simpler?

Basically I have a table of around 100 plus rows, with the a,b,c and d as the column. Based on a,b,c and d, I need to perform 3 different type of function.

The table describes a set a business rules.

 uint8 a;
 uint8 b;
 uint16 c;
 uint8  d;

 if      ( a == 1 &&   b == 1           && c == 0)           { functionA();}
 else if ( a == 5 &&   b == 5           && c == 2 && d == 2) { functionB();}
 else if ( a == 1 && (b ==36 || b == 7) && c == 0)           { functionC();}  
 else if ( a == 3 &&   b == 3                     && d == 50) {functionA();}
    :
    :

Answer 1

Dreamt about this question overnight .. and have come up with a neat solution (inspired by the matching systems used in the google test ilbraries)

The core if mess becomes something like this - which I think is quite pretty.

  Params(1,2,3,4)
    .do_if( match(1,_,3,5), functionA )
    .do_if( match(1,_,3,4), functionB )
    .do_if( match( _, OR(2,3),3,5), functionC )
//    .do_if( match(1,_,4,6)|match(3,_,5,8) ), functionD )
    ;

Final line I haven't implemented yet. _ means match any digit, OR means match either (though you can nest it OR(1,OR(2,3)) should be fine.

The rest of the support code is a mess of template functions to make this work. If there's interest I can post a more thorough description of what's going on .. but its not overly complicated - just long. I expect it can be cleaned up a bit too...

It can probably be pulled out and generalized into a nice library too - though I'd probably look at adapting the google test code instead of basing anything off this code ;)

struct RawParams
{
  RawParams( int a, int b, int c, int d) : a_(a), b_(b), c_(c), d_(d) {}
  int a_,b_,c_,d_;
};

struct ParamsContinue
{
  RawParams * p_;

  ParamsContinue() : p_(0) {}
  ParamsContinue( RawParams * p ) : p_(p) {}

  template<typename CONDITION, typename FN>
  ParamsContinue do_if( CONDITION cond, FN fn )
  {
    if( !p_ ) { return ParamsContinue(); }
    if( cond(p_->a_,p_->b_,p_->c_,p_->d_) ) { fn(); return ParamsContinue(); }
    return *this;
  }
};

struct Params
{
  Params( int a, int b, int c, int d) : params_(a,b,c,d) {}
  RawParams params_;

  template<typename CONDITION, typename FN>
  ParamsContinue do_if( CONDITION cond, FN fn )
  {
    return ParamsContinue(&params_).do_if(cond,fn);
  }
};

struct AnySingleMatcher
{
  bool operator()(int i) const { return true; }
};

AnySingleMatcher _;

template<typename M1, typename M2, typename M3, typename M4>
struct Match
{
  Match( M1 in_m1, M2 in_m2, M3 in_m3, M4 in_m4 ) : 
    m1(in_m1),
    m2(in_m2),
    m3(in_m3),
    m4(in_m4)
  {}

  bool operator()( int a, int b, int c, int d) const { return m1(a)&&m2(b)&&m3(c)&&m4(d); }

  M1 m1;
  M2 m2;
  M3 m3;
  M4 m4;
};

struct AnyMatcher {};
struct IntMatcher
{
  IntMatcher(int i) : i_(i) {}
  bool operator()(int v) const { return v==i_; }
  int i_;
};

template<typename T>
struct as_matcher
{
  typedef T type;
  static T as( T t ) { return t; }
};

template<>
struct as_matcher<int>
{
  typedef IntMatcher type;
  static IntMatcher as( int i ) { return IntMatcher( i ); }
};

template<typename M1, typename M2, typename M3, typename M4 >
Match< typename as_matcher<M1>::type, typename as_matcher<M2>::type, typename as_matcher<M3>::type, typename as_matcher<M4>::type >
match( M1 m1, M2 m2, M3 m3, M4 m4 )
{
  return 
    Match< typename as_matcher<M1>::type, typename as_matcher<M2>::type, typename as_matcher<M3>::type, typename as_matcher<M4>::type >(
      as_matcher<M1>::as(m1), as_matcher<M2>::as(m2), as_matcher<M3>::as(m3), as_matcher<M4>::as(m4) );
};

template<typename T1, typename T2>
struct OrMatcher
{
  OrMatcher( T1 t1, T2 t2 ) : t1_(t1), t2_(t2) {}
  T1 t1_;
  T2 t2_;
  bool operator()(int i) const { return t1_(i) || t2_(i); }
};

template<typename T1, typename T2>
OrMatcher< typename as_matcher<T1>::type, typename as_matcher<T2>::type > OR( T1 t1, T2 t2 )
{
  return OrMatcher< typename as_matcher<T1>::type, typename as_matcher<T2>::type >( as_matcher<T1>::as(t1),as_matcher<T2>::as(t2) );
};

#include <iostream>
void functionA(){ std::cout<<"In A"<<std::endl;};
void functionB(){ std::cout<<"In B"<<std::endl;};
void functionC(){ std::cout<<"In C"<<std::endl;};
void functionD(){ std::cout<<"In D"<<std::endl;};

int main()
{
  Params(1,2,3,4)
    .do_if( match(1,_,3,5), functionA )
    .do_if( match(1,_,3,4), functionB )
    .do_if( match( _, OR(2,3),3,5), functionC )
//    .do_if( match(1,_,4,6)|match(3,_,5,8) ), functionD )
    ;

}

Answer 2

To expand Tony's first point:

you can populate a map from a struct holding a, b, c, and d values to check for to the function to call

Wrap all of your variables up in a state object or something:

struct state {
    uint8 a;
    uint8 b;
    uint16 c;
    uint8 d;
}

And add a bunch of those possible states to lists:

std::set<state> functionASet;
functionASet.insert(aState);
...

Then test whether the set contains a state constructed of the current values for a, b, c, d:

// init a state struct with current values for a, b, c, d
if(functionASet.find(currentValues) != functionASet.end())
    functionA();
else if(functionBSet.find(currentValues) != functionASet.end())
    functionB();
else ...

OR, add the states to a map:

typedef void (*func)();

std::map<state, func> functionMap;

And simply call the function which matches the found state:

std::map<state, func>::iterator search = functionMap.find(currentValues);
if(search != functionMap.end())
    (search->second())();

Answer 3

It seems really messy :/

When you need to describe business rules, you should use a descriptive approach rather than an imperative approach. It is much more readable and it is usually much easier to adapt the rules.

My first thought is to use a table, indexed by a, b, c and d, and have function pointers (or functors) within.

The initialization code will be a bit frightening, my advice would be to keep it ordered lexicographically:

// Note: you don't have to initialize the null values if the variable is static
Table[0][0][0][1] = functionA;
Table[0][3][0][1] = functionB;
...

Retrieving the function is then a piece of cake, just remember to test for nullity if it's possible that there is no function (and assert otherwise).

Another solution would be to break down the choice into steps, using functions:

• switch on a, choose the function to call (use the default to handle the case where you do not care about a)
• switch on b, c, d ....

Example:

void update(int a, int b, int c, int d) {
  switch(a) {
  case 0: updateA0(b, c, d); break;
  case 1: updateA1(b, c, d); break;
  default: updateAU(b, c, d); break;
  }
}

void updateA0(int b, int c, int d) {
  switch(b) {
  case 0: updateA0B0(c, d); break;
  case 1: updateA0B1(c, d); break;
  default: updateA0BU(c, d); break;
  }
}

// etc...

It makes it easier to "follow" the update chain, and to apply local updates. Also, the logic being proper to each subfunctions, it's easy to apply ranges selections (if (b >= 5 && b < 48)) without "breaking" the pattern or duplicating the initialization. Finally, depending on the likeliness on some paths, you can easily choose to switch on d first in updateA1 if it makes it easier.

It is at least as flexible as your current solution, but much more readable/maintainable.

Answer 4

There are lots of ways to make it simpler, for example:

• you can populate a map from a struct holding a, b, c, and d values to check for to the function to call (the code to populate the map may still be a mess, but it'll be faster and cleaner executing; can add two keys for cases ala b == x || b == y)
• you can manually factor the conditions: given your example, if (a == 1) if (b == 1 && c == 0) functionA(); else if ((b == 36 || b == 7) && c == 0) functionC();. switch statements may make this cleaner. In such a factoring, you can also use <, <=, >, and/or >= to divide larger search spaces, improving performance to from O(N) to O(log2N).
• for the common simple case of testing a, b, c, and d once, use a macro ala #define ELIF_ABCD(A, B, C, D, F) else if (a == (A) && b == (B) && c == (C) && d == (D)) F();. Add macros as necessary for other combinations of tests e.g. ABD, ABC, AD.
• (may make the code more cryptic), but could explore bit-shifting and ORing together the values into a large enough type (int64_t) then e.g. binary search an array of function pointers

Something to look out for though is that the if/else chain may contain things like:

if (a == 1 && c == 3 && d == 2) functionY();
else if (a == 1 && b == 2 && c == 3) function X();

Here, the order is significant as an input can match both. This aspect can easily get lost or altered if the searches are factored differently or some manner of indexing to function pointer is used, which is one argument in favour of the macro approach above.

Answer 5

To do this correctly and efficiently, you first need to standardize the representation of each row and transform it into compact data that can be indexed or sorted. You could try doing this by simply serializing the values of the columns into a fixed-length string, and then inserting this string and a pointer for the appropriate function into a map with the string as the key and the function pointer as the value.

However, the problem is a bit more complex because in some rows some columns do not count, they are "don't cares." Assuming there is no value in each column that can act as a "dont care" value, in addition to values for each column, the key also must contain data indicating which columns are significant. We can do this by appending an extra byte to the string that contains a bit mask indicating which columns are significant. For map searching to work correctly in this case, insignificant columns must always contain the same value in the key (zero is a good choice).

Now we have a only to write a function to construct a six-byte key from the columns of each row of our table. Use this function to do the initial map inserts and the lookups after the map is built.

This method is quite fast for lookups, O(log n), where n is number of rows.

Answer 6

Without further details, one can only guesss as to how to simplify it.

One possibility is the use of boolean variables. If you are constantly evaluating certain combinations, you could save that reevaluation by using booleans.

If there are a fixed set of conditions, you could also use a bit mask against an int and then do a case.

But again, these are just guesses without knowing more details about what you are really doing.

Answer 7

I'd consider something like this - which keeps the conditions with the functions and makes the whole lot much easier to test and extend (In my opinion).

You can probably produce subclasses that take constructor parameters to reduce the total number of classes required.

class ICase
{
  virtual ~ICase() {}
  virtual bool matches_arguments( int a, int b, int c ) const =0;
  virtual void do_it( int a, int b, int c)=0;
};

class CaseA : public ICase
{
  bool matches_arguments( int a, int b, int c ) const { return ( a == 1 &&   b == 1           && c == 0); }
  bool do_it(int a, int b, int c) { functionA(); }
};

...
//Some setup - only need to do this once
std::vector< shared_ptr<ICase> > cases;
cases.push_back( new CaseA );
cases.push_back( new CaseB );

//The conditionals
for( int i=0; i<cases.size(); ++i)
{
  if( cases[i]->matches_arguments(a,b,c) ) { cases[i]->do_it(a,b,c); break; }
}

Answer 8

Following from Tony's suggestion to use a map you could probably optimize it a bit.

You could encode all 4 of the numbers as a single uint64_t (or smaller depending on the range of their values).

uint64_t h = (a << 32) + (b << 24) + (c << 8) + d;

You could then build a std::map<uint_64_t, void (*)()> that maps the hash to a function pointer. It may take some effort to construct the map though. I think it would be better for you to listen to everyone the other suggestions and refactor your code.

Answer 9

Split it up based on your 4 variables

if(a==1)
{
    if(b==1)
    {

    }
    else if(b==3)
    {

    }
}
else if(a==3)
{

}

that would make it a little simpler to read and follow