[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 485
  • Last Modified:

Simplify C++ statement block (flowchart output)

I WANT TO BEAUTIFY MY CODE AND AVOID COMPLEXITY AND NESTED STATEMENTS AS MUCH AS POSSIBLE.

I have a flowchart and converted it to code the way below. Now it seems that it is not in a friendly and readable format. It has some nested statements that are less easy to follow.

Can anyone suggest a way to simplify it to a most readable format. Also it is much appreciated if anyone could provide a GENERAL GUIDELINE FOR SIMPLIFYING C++ CODE and keeping it readable (specially when CONVERTING FROM A PRE-DESIGNED FLOWCHART which may lead to deep nested structures if directly implemented)

Many Thanks in Advance,

Here is the sample code (compilable):
---------------------------------
void block1(){
}

void block2(){
}

void flowchart(){
      bool changed = true;
      bool dbChange = true;
      int changeType = 4;
      bool deviceChange = true;
      bool override = true;
      bool active = true;

      if (changed){
            if (changeType==1/*s*/){
            }else if (changeType==2/*d*/){
                  if (override/*h2d*/){
                  }else{
                        block1();
                  }
            }else{
                  if (active){
                        block1();
                  }else{
                        block2();
                  }
            }
      }else{
                  if (active){
                        block1();
                  }else{
                        block2();
                  }
      }

}
---------------------------------
0
Al fa
Asked:
Al fa
  • 6
  • 5
2 Solutions
 
Infinity08Commented:
This is shorter, and I think clearer (you decide) :

      if (changed && ((changeType == 1) || (changeType == 2))) {
          if ((changeType == 2) && (!override)) {
              block1();
          }
      }else{
          if (active){
              block1();
          }else{
              block2();
          }
      }

You could also make use of switches if you have more than 2 changeTypes.

A general guideline would be to optmize for readability, and let the compiler optimize for performance.

In the example you posted, you had twice this block :

          if (active){
              block1();
          }else{
              block2();
          }

These could easily be combined since they were both in else blocks.

Furthermore, a few quick tips :

1) You had 2 empty if blocks ... try to avoid that - it makes reading the code more difficult, and creates confusion. In other words, this :

          if (condition) {
          }
          else {
              do_something();
          }

is equivalent to :

          if (!condition) {
              do_something();
          }

which is a lot clearer.


2) Another easy fix is nested if's like this :

          if (condition) {
              if (condition2) {
                  do_something();
              }
          }

is the same as :

          if (condition && condition2) {
              do_something();
          }


3) Two if's that have the same block body can be combined, so this :

          if (condition) {
              do_something();
          }
          else if (condition2) {
              do_something();
          }

is the same as :

          if (condition || condition2) {
              do_something();
          }



And a lot more.
0
 
alexcohnCommented:
In your case, consider simply defining all possible execution paths, like here:

if (changed && changeType==2/*d*/ && !(override/*h2d*/))
{
   block1();
};
if (changed &&  changeType !=1 && changeType != 2 && active)
{
  block1();
};
if (changed &&  changeType !=1 && changeType != 2 && !active)
{
  block2();
};
if (!changed && active)
{
      block1();
};
if (!changed && !active)
{
       block2();
};

I believe it's easy to automate generation of such text, because the underlying logics is very straightforward.

These could be aggregated, but at the price of simplicity (in my humble opinion):

if ( (changed && changeType==2/*d*/ && !(override/*h2d*/)) || (changed &&  changeType !=1 && changeType != 2 && active) || (!changed && active) )
{
  block1();
}
if ( (changed &&  changeType !=1 && changeType != 2 && !active) || (!changed && !active) )
{
  block2();
}

Actually, boolean expression analysis can be applied to the latter, to produce something like:

if ( (changed && (changeType==2/*d*/ && !(override/*h2d*/)) || (changeType !=1 && changeType != 2 && active)) || (!changed && active) )
{
  block1();
}
if (!active && ((changed && changeType !=1 && changeType != 2 ) || (!changed))
{
  block2();
}
0
 
Infinity08Commented:
>> In your case, consider simply defining all possible execution paths, like here:

I wouldn't say that's easier to read ...
0
Concerto's Cloud Advisory Services

Want to avoid the missteps to gaining all the benefits of the cloud? Learn more about the different assessment options from our Cloud Advisory team.

 
alexcohnCommented:
> I wouldn't say that's easier to read ...

You are right, it all depends on the purpose. If a person wants to follow the flow logic, your version is much better, but still it is worse than looking at the graphical chart. If a person wants to know what causes execution of each block, my version gives a direct answer. It goes side-by-side with the original flowchart, not attempting to replace it.

Consider another formatting of the same code:

if ( (changed && changeType==2 && !override ||
     (changed && changeType !=1 && changeType != 2 && active) ||
     (!changed && active) )
{
   block1();
};
if ( (changed &&  changeType !=1 && changeType != 2 && !active) ||
      (!changed && !active) )
{
       block2();
};
0
 
Infinity08Commented:
>> Consider another formatting of the same code:

That's better, and I do see your point. I'll leave it up to ali_fakoor to decide which one he thinks is easier to read. Because ultimately we're trying to help him :)

btw, alexcohn, those ;'s after your if blocks should not be there !
0
 
alexcohnCommented:
What's wrong with the semicolons? Note that my whole point is to avoid
if {} else if {} else {};
0
 
Infinity08Commented:
A valid if statement is :

    if (condition) {
        do_something();
    }

Notice that there is NO ; after the block. Same goes for if-else statements :

    if (condition) {
        do_something();
    }
    else {
        do_something_else();
    }

Adding a ; after that would mean an empty statement, and that doesn't do anything. It only confuses people.
There are a few cases where it is valid to use an empty statement (called null statement in the standard), but this is not one of them.

It can be valid if you want to end a loop without a body :

    char *s;
    /* ... */
    while (*s++ != '\0')
        ;

or if you want to add a label just before the end of a block :

    while (loop1) {
        /* ... */
        while (loop2) {
            /* ... */
            if (want_out)
                goto end_loop1;
            /* ... */
        }
        /* ... */
    end_loop1: ;
    }

(examples taken from the specification).

There's no point in using a null statement after an if block though.
0
 
alexcohnCommented:
You are right in that the semicolon is not necessary after the curly bracket. The main reason I used it was to emphasize that the if () {} statement ends and is not chained with else if () {} else if () {} ....

Yet I will be obliged to be presented with a compiler that produces different code for
     if (a == b) { c(); }
and
     if (a == b) { c(); };
or, if you we are on this subject,
     if (a == b) c();

Which reminds me that all curly brackets in my version are redundant.
0
 
Infinity08Commented:
>> Yet I will be obliged to be presented with a compiler that produces different code >> for
>>      if (a == b) { c(); }
>> and
>>      if (a == b) { c(); };

The only difference can be the addition of NOP commands (which should be removed in the optimization stage anyway). If your compiler generates code with different behavior for the above two fragments, then better get a standards compliant compiler !!

curly brackets should be kept at all times, because it avoids problems in the future. Even if the if body is empty or only a single statement, it's still highly advised to add the curly brackets anyway.
0
 
alexcohnCommented:
My question was about the possible NOP commands - I have never seen a non-optimizing compiler that added these. And if the difference is purely aesthetical, I claim that having the semicolon in the end helps - in that there is no confusion with if () {} else if {} chain.
0
 
Infinity08Commented:
>> I claim that having the semicolon in the end helps - in that there is no confusion with if () {} else if {} chain.

That's your good right, and I'm not gonna argue about it. I've already spent too much time on this :)
0
 
Al faAuthor Commented:
Wow ,
Thanks,

Great informative conversation.
I need some more  time to study on these and decide which feels convenient for me.

Many Many Thanks,
Ali.
0

Featured Post

Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 6
  • 5
Tackle projects and never again get stuck behind a technical roadblock.
Join Now