Great if else expression

Question

If you have a big (about 100 plus) if else, as shown below, and the if else condition can be irregular (for example, some of them depend on 3 variables, some of them depend on 4), is there a way to simplify it?

Basically, I have a table of about 100 rows, and in the column a, b, c and d. Based on a, b, c and d, I need to execute 3 different types of functions.

The table describes a set of 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

There are many ways to make this easier, for example:

• you can fill the card from the values ​​of the structure a , b , c and d to check the function to be called (the code to fill the card can still be messy, but it will be faster and cleaner, can add two keys for cases ala b == x || b == y )
• you can manually determine the conditions: given your example, if (a == 1) if (b == 1 && c == 0) functionA(); else if ((b == 36 || b == 7) && c == 0) functionC(); if (a == 1) if (b == 1 && c == 0) functionA(); else if ((b == 36 || b == 7) && c == 0) functionC(); . Switch switch can make this cleaner. With this factoring, you can also use < , <= , > and / or >= to separate large search spaces, increasing performance from O (N) to O (log2N).
• for the usual simple case of testing a , b , c and d use the ala #define ELIF_ABCD(A, B, C, D, F) else if (a == (A) && b == (B) && c == (C) && d == (D)) F(); macro once #define ELIF_ABCD(A, B, C, D, F) else if (a == (A) && b == (B) && c == (C) && d == (D)) F(); . Add the macros needed for other test combinations, for example. ABD , ABC , AD .
• (can make the code more mysterious), but can examine the bit offset and ORing values ​​together in a sufficiently large type ( int64_t ), and then, for example, binary search for an array of function pointers

Something you need to know is that the if / else chain can 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, since the input can correspond to both. This aspect can be easily lost or changed if the search is taken into account in different ways or some kind of pointer to a function pointer is used, which is one of the arguments in favor of the above macro.

Answer 2

Following Tony’s suggestion to use the map, you can probably optimize it a bit.

You can encode all 4 numbers as one uint64_t (or less depending on the range of their values).

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

Then you can build std::map<uint_64_t, void (*)()> , which maps the hash to a function pointer. However, it may take some time to create the map. I think you would be better off listening to all the other suggestions and reorganizing your code.

Answer 3

Separate it based on four variables

 if(a==1) { if(b==1) { } else if(b==3) { } } else if(a==3) { } 

which would make it a little easier to read and follow

Answer 4

I would think of something like this, which preserves conditions with functions and makes testing and extending the whole task easier (in my opinion).

You might create 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 5

To expand Tony's first point:

you can populate a map from a structure containing the values ​​a, b, c and d to check the function to call

Wrap all your variables in a state object or something else:

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

And add a list of possible states to the list:

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

Then check if the set contains a state built from 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 states to the map:

 typedef void (*func)(); std::map<state, func> functionMap; 

And just call the function that matches the found state:

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

Answer 6

To do this correctly and efficiently, you first need to standardize the presentation of each row and convert it to compact data that can be indexed or sorted. You can try to do this by simply serializing the column values ​​into a fixed-length row, and then inserting that row and the corresponding function pointer into the map with the row as the key and the function pointer as the value.

However, the problem is a little more complicated, because in some rows some columns are not taken into account, they are "not taken care of". Assuming that each column does not have a value that can act as the “do not care” value, in addition to the values ​​for each column, the key should also contain data indicating which columns are significant. We can do this by adding an extra byte to the line containing the bitmask indicating which columns are significant. For a correct map search in this case, insignificant columns should always contain the same value in the key (zero is a good choice).

Now we only have to write a function to build a six-byte key from the columns of each row of our table. Use this function to create original map inserts and search after creating a map.

This method is pretty fast for searching, O (log n), where n is the number of lines.

Answer 7

I dreamed about this issue overnight .. and came up with a neat solution (inspired by the corresponding systems used in google test tags)

The kernel, if the mess becomes something like this, which I think is pretty 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 ) ; 

The last line has not yet been implemented. _ means match any digit, OR means match (although 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 I have an interest, I can post a more detailed description of what is happening ... but its not too difficult - just a long time. I expect it can be cleaned too ...

It can probably be pulled out and generalized into a beautiful library too, although I would probably look at adapting the google test code instead of doing something from it;)

 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 8

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

One possibility is to use boolean variables. If you constantly evaluate certain combinations, you can save this reevaluation using booleans.

If there is a fixed set of conditions, you can also use a bitmask against an int , and then make a case .

But then again, it's just a hunch, not knowing the details of what you are really doing.

Answer 9

Seems really messy: /

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

My first thought is to use a table indexed with a, b, c and d, and have pointers to functions (or functors) inside.

The initialization code will be a little intimidating, my advice will be to arrange it 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; ... 

Extracting a function is a piece of cake, just remember to check for nullity if it is possible that there is no function (and assert otherwise).

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

• enable a , select the function to call (use default to handle the case when you don't care about a )
• include 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... 

This makes it easy to “follow” the update chain and apply local updates. In addition, the logic corresponding to each subfunction easily applies the choice of ranges ( if (b >= 5 && b < 48) ) without "breaking" the template or duplicating initialization. Finally, depending on the likelihood on some paths, you can easily include d first in updateA1 if that makes it easier to do.

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