If-else-if refactoring structure

Question

How can I refactor the following C ++ code? I am using C ++ 11 in my program

void f(int a, int b, int c, int d, int e, int f) { // MAX1..MAX6 are constants, N1..N6 are constants if( a > MAX1) { .. code block 1.. } else if(b > MAX2) { .. code block 2.. } else if(c > MAX3) { .. code block .. } else if(d > MAX4) { .. code block 3 .. } else if(e > MAX5) { .. code block 4 .. } else if(f > MAX6) { .. code block5 .. } else if(( a > N1) && ( b > N2)) { .. code block 6 .. } else if(( c > N3) && ( d > N4)) { .. code block 7 .. } else if (( e > N5) && ( f > N6)) { .. code block 8 .. } else if (( a > N1) && ( b > N2) && (d > N4)) { .. code block 9 .. } else if (..some combination of (a > N1) to (f > N6)...) { // there are more than 30 such combinations in my code .. code block XX .. } else { .... code block .... } 

Answer 1

A function will have no more than 64 code blocks, each of which can use an anonymous function or a named one:

 my_func_array[64] = { [CONDITION(1,0,0,0,0,0)] = { codeblock1 }, [CONDITION(0,1,0,0,0,0)] = { codeblock2 }, ... }; 

The macro basically combines the 6 first entries in the index, essentially translating into:

  my_func_array[64] = { [32] = { ... }, [16] = { ... }, }; 

means you do not need to enter conditions in any particular order ...

At startup, you should also evaluate all the conditions:

  int condition = CONDITION(a < MAX1, b < MAX2, c < MAX2, ...); if (my_func_array[condition]) my_func_array[condition](); else { // this second block should cover all the other cases int condition2 = CONDITION(a < N1, b < N2, c < N3, ... ); if (my_func_array2[condition2]) my_func_array2[condition2](); } 

Answer 2

What you have does not seem unreasonable, but if you really want to reorganize, you can create an enumeration

enum class CaseSelect {CASE0, CASE1, CASE2};

then create a function that looks very similar to your current if tree, with each block of code returning the corresponding enumeration.

Then create a case statement using enumerations with the appropriate logic in each

It will not bring you much, but it separates the state selection logic from the operational logic, which may be useful for clarity in the case of multi-line statements.

Answer 3

You can start by creating a through f , MAX and N into arrays (just an initial sketch, which I can expand with additional information about your conditions, code blocks and their use):

 const int MAX[6] = { ... }; const int N[6] = { ... }; const std::function funcs[6]; void f(int in[6]) { for(int i = 0; i < 6; ++i) { if(in[i] > MAX[i]) { funcs[i](); break; } } } 

Answer 4

So, let's say you have a discrete number N of mutually exclusive integer ranges (xi to yi), and you want to call some block of code depending on which range is given for input z, and then:

• you want to use the binary tree std::map to store the beginning of each range and the corresponding lambda.
• call std::lower_bound on z to find a range of candidates
• then check the upper bound of this range at z.
• if inside, call lambda

This will give O (log (N)) time compared to O (n) for a large if-else chain.

Answer 5

First, I would get rid of the first 6 branches like Mark B , but

• instead of break I would return ,

• I would just use function pointers, std::function is the excess here,

• I would use std::array and bind the check instead of raw arrays.

Now that the boring cases are gone, I would apply the same pattern to the remaining branches, but it requires some tricking, as these conditions are composites.

I assume that MAX1 > N1 . Hope it's true.

First, I would build a decimal number by encoding all the arguments:

 int arg=10^5*(a>N1?1:0)+10^4*(b>N2?1:0)+10^3*(c>N3?1:0)+10^2*(d>N4?1:0)+10*(e>N5?1:0)+(f>N1?1:0) 

I would also code the conditions in a similar way. For example: the condition (a > N1) && ( b > N2) becomes arg >= 110000 , etc.

As you can see, after this encoding, you can delete the remaining branches in the same way as the first 6 branches.

If efficiency is a serious problem, you can do the same trick with the powers of two (bits, then use bitmasks). I do not understand the bit, so I can not help it, but it works the same way and it is more efficient. It is probably as effective as having such a long if-else if chain that you have now, or maybe even more efficient, because you are not repeating the same comparisons.

Hope this helps.

Answer 6

If there are many rules, there may be function objects (for example, lambdas) to save:

I simplified this a bit for demonstration purposes, only 3 arguments ...

And I assume that there is a safe value for non-validation, in this case 0 (it could live without it, but then it would look ugly).

 #include <iostream> #include <functional> struct Rule { int a; int b; int c; std::function<int()> fun; }; Rule rules[]{ { 10, 0, 0, []() { std::cout << "First!"; return 0;} }, { 0, 20, 0, []() { std::cout << "Second!"; return 1;} } }; int f(int a, int b, int c) { for (Rule rule : rules) { if ((rule.a == 0 || a > rule.a) && (rule.b == 0 || b > rule.b) && (rule.c == 0 || c > rule.c)) return rule.fun(); } std::cout << "Not match!"; return 2; } int main() { f(5, 23, 3); };