making nested if-else more manageable and efficient

Question

I am working on a project which analyzes and predict wave patterns, but the thing is that the code of nested if/else is becoming more harder to manageable, here is snippet:

for (int x = 1; x <= 24; x++)            
{
if ((15 <= Angles[x]) && (Angles[x] <= 30))
  {
   if(x<=6)
    {
      counter=counter+4;
      if (edges[x] < edges[x+2]) { counter = counter - 4; }
      else{counter=counter+5; };
      if (Angles[x] < Angles[x + 2]) { counter = counter - 6; }
      else{counter=counter + 4; };
      if ((edges[x+1] - edges[x]) < (edges[x + 3] - edges[x + 2])) { counter= counter - 6; }
      else{counter-counter + 5; };
      if (gradient[x] < gradient[x + 2]) { counter = counter - 6; }
      else{counter=counter + 5; };
    };

if((7<=x)&&(x<=12))
{
  counter = counter - 10;
  if (edges[x] < edges[x+2]) { counter = counter - 5; }
  else{counter=counter + 6; };
  if (Angles[x] < Angles[x + 2]) { counter = counter - 7; }
  else{counter=counter + 6; };
  if ((edges[x+1] - edges[x]) < (edges[x + 3] - edges[x + 2])) { counter = 
  counter - 6; }
  else{counter-counte+7; };
  if (gradient[x] < gradient[x + 2]) { counter = counter - 6; }
  else{counter=counter+8; };
};

if((13<=x)&&(x<=18))
{
  counter = counter - 30;
   if (edges[x] < edges[x+2]) { counter = counter - 7; }
   else{counter=counter + 6; };
   if (Angles[x] < Angles[x + 2]) { counter = counter - 7; }
   else{counter=counter + 5; };
   if ((edges[x+1] - edges[x]) < (edges[x + 3] - edges[x + 2])) { counter = 
   counter - 7; }
   else{counter-counter+5; };
   if (gradient[x] < gradient[x + 2]) { counter = counter + 5; }
   else{counter=counter + 5; };
};

//there are more waves classification and weightage to come//
if ((30 <= Angles[x]) && (Angles[x] <= 45)){}.....
if ((45 <= Angles[x]) && (Angles[x] <= 60)){}.....
if(60 < Angles[x]){}....

};

Angles is a container storing wave angle values. edges are peak and trough of waves. gradient contains the gradient value of wave counter is the weightage of the current pattern. values are stored in serial manner in a vector.

This is just a rough stripped down snippet (ignore syntax error if you find one). Each pattern consists of a for loop with nested if/else within it. There are 13 more patterns.

I have already crossed 5000 lines just writing those if/else for loops. Can you guys help me with making this more manageable?

You must use if else, because if value of x is less than 6, it's unnecessary to check x for other cases — Mesar ali, Apr 23 '18 at 05:47
In this specific cases, you could replace `if`/`else` with the [ternary condition operator](https://en.wikipedia.org/wiki/%3F:) `?:`. E.g. instead of `if (edges[x] < edges[x+2]) { counter = counter - 4; } else{counter=counter+5; };` you could write: `counter = edges[x] < edges[x+2] ? counter - 4 : counter + 5;` or even: `counter += edges[x] < edges[x+2] ? -4 : 5;` — Scheff's Cat, Apr 23 '18 at 06:18
Except from other typos: Are you sure this is what you intended: `else{counter-counter + 5; };` — Andreas H., Apr 23 '18 at 07:04

Andreas H. · Accepted Answer · 2018-04-23T10:31:17.670

Instead of

   if(x<=6)
    {
      counter=counter+4;
      if (edges[x] < edges[x+2]) { counter = counter - 4; }
      else{counter=counter+5; };
      if (Angles[x] < Angles[x + 2]) { counter = counter - 6; }
      else{counter=counter + 4; };
      if ((edges[x+1] - edges[x]) < (edges[x + 3] - edges[x + 2])) { counter= counter - 6; }
      else{counter-counter + 5; };
      if (gradient[x] < gradient[x + 2]) { counter = counter - 6; }
      else{counter=counter + 5; };
    };

you could write

   if(x<=6)
    {
      counter = counter
        + 4
        + ((edges[x] < edges[x+2]) ? -4 : 5)
        + ((Angles[x] < Angles[x + 2]) ? -6 : 4)
        + (((edges[x+1] - edges[x]) < (edges[x + 3] - edges[x + 2])) ? -6 : 5)
        + ((gradient[x] < gradient[x + 2]) ? -6 : 5)
    };

If you could put your conditions in there own function the code could become clearer too.

I see many conditions like array[x] < array[x+2]. You can define lambdas like

auto raisingEdge = [&](int x) { return edges[x] < edges[x+2]; };

and use it like

counter += raisingEdge(x) ? -4 : 5;

This could make your Code even more readable:

  if(x<=6)
    {
      counter = counter
        + 4
        + (raisingEdge(x) ? -4 : 5)
        + (raisingAngle(x) ? -6 : 4)
        + (raisingEdgeDelta(x) ? -6 : 5)
        + (raisingGradient(x) ? -6 : 5)
    };

(The names are just to show the concept; I have no idea nor do I care what these values really are)

thank you its very helpful , its easier to read and understand — chronus, Apr 23 '18 at 07:28

HMD · Answer 2 · 2018-04-23T07:20:37.983

2

You also can use lambda with condition ? if : else semantic.

for your first couple if statements you can do something like this :

auto comp = [&counter](int n, int m, int val1, int val2) { return n < m ? counter + val1 : counter + val2; };
counter = comp(edges[x], edges[x + 2], -4, 5);
counter = comp(Angles[x], Angles[x + 2], -6, 4);

it's the same for your other ifs too and you can add them.

edited Apr 23 '18 at 07:20

answered Apr 23 '18 at 07:12

HMD

2,202
6
24
37

If you remove the capture of "counter", return only the offsets and use counter += comp() the lambda would be as simple as a global function not capturing anything. – Andreas H. Apr 26 '18 at 05:58

Basile Starynkevitch · Answer 3 · 2018-04-23T08:22:44.360

I am not sure that your small sequence of if ... else if is unmanageable or inefficient (IMHO it is manageable and efficient, but requires a better indentation; consider reformatting your code with tools like astyle).

In addition of other answers, with GCC and compatible (e.g. Clang) compilers, you might consider using some language extension:

a switch using case ranges (this has my preference, and is very readable), so something like

switch(x) { 
   case 1 ... 6:
    {
      counter=counter+4;
      if (edges[x] < edges[x+2]) { counter = counter - 4; }
      else {counter=counter+5; };
      if (Angles[x] < Angles[x + 2]) { counter = counter - 6; }
      else{counter=counter + 4; };
      if ((edges[x+1] - edges[x]) < (edges[x + 3] - edges[x + 2])) 
        { counter= counter - 6; }
      else{counter-counter + 5; };
      if (gradient[x] < gradient[x + 2]) { counter = counter - 6; }
      else{counter=counter + 5; };
   };
   break;

a computed goto using labels as values

Of course, you need to make (consciously) the decision to use a language extension that the C++11 (see n3337) or C++14 standards do not define. (The above code won't even compile on some Microsoft compilers, because it is non-standard).

If using GCC and/or Clang, don't forget to enable all warnings and debug info, e.g. to pass -Wall -Wextra -g to g++ or clang++

i've already crossed 5000 lines just writing those if/else for loops .

You could decide to refactor your code by defining small specific internal functions (perhaps static ones, or private member functions) to handle each case. As rule of thumb, try to avoid huge hand-written functions of many thousand lines: define smaller internal functions of at most a few hundreds lines each, and call them appropriately - perhaps only once. It is often (but not always) useful to put a chunk of a few hundred lines in a function, even if that internal function is called only once. It is up to you to organize that wisely, and to give meaningful names to your internal functions.

Coding in C++ is also a matter of (many) conventions and habits. I recommend studying the source code of some free software similar (in goals, in spirit) to yours for inspiration. (you'll find many free software projects in C++ on github, sourceforge, and elsewhere).

In some cases, you could even consider generating your C++ code from something else (higher level, specific to your approach). Yo might want to generate even #line directives in it. Perhaps such a metaprogramming approach could be worthwhile in your case (be sure to read about ASTs before even trying).

Melebius · Answer 4 · 2018-07-02T08:04:58.803

Each block (starting with counter = counter … line) seems to be same except for numerals. In this case, I’d definitely go for a function. The numerals will become its parameters. If you make it a method of the same class and the variables used in the blocks are class properties, the new method can access them easily.

This would definitely help the manageability of your code. In the case you discover an error in the calculation, you’ll only have to correct one place.

A rule of thumb: If you are copy-pasting some part of your code more than once, you should think about making it an entity that can be used repeatedly, like a macro, function or class.

See also: Wikipedia – Don't repeat yourself

By the way,

counter = counter - 10;

(and all similar cases) can be shortened to

counter -= 10;

making nested if-else more manageable and efficient

4 Answers4