Solved

Simplify C++ statement block (flowchart output)

Posted on 2007-04-10
12
473 Views
Last Modified: 2013-12-17
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
Comment
Question by:Ali Fakoor
  • 6
  • 5
12 Comments
 
LVL 53

Accepted Solution

by:
Infinity08 earned 125 total points
Comment Utility
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
 
LVL 11

Assisted Solution

by:alexcohn
alexcohn earned 125 total points
Comment Utility
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
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> In your case, consider simply defining all possible execution paths, like here:

I wouldn't say that's easier to read ...
0
 
LVL 11

Expert Comment

by:alexcohn
Comment Utility
> 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
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> 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
 
LVL 11

Expert Comment

by:alexcohn
Comment Utility
What's wrong with the semicolons? Note that my whole point is to avoid
if {} else if {} else {};
0
Free Trending Threat Insights Every Day

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
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
 
LVL 11

Expert Comment

by:alexcohn
Comment Utility
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
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> 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
 
LVL 11

Expert Comment

by:alexcohn
Comment Utility
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
 
LVL 53

Expert Comment

by:Infinity08
Comment Utility
>> 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
 
LVL 5

Author Comment

by:Ali Fakoor
Comment Utility
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 Trending Threat Insights Every Day

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

Suggested Solutions

I use more than 1 computer in my office for various reasons. Multiple keyboards and mice take up more than just extra space, they make working a little more complicated. Using one mouse and keyboard for all of my computers makes life easier. This co…
Healthcare organizations in the United States must adhere to the guidance of both the HIPAA (Health Insurance Portability and Accountability Act) and HITECH (Health Information Technology for Economic and Clinical Health Act) for securing and protec…
The viewer will learn how to successfully download and install the SARDU utility on Windows 7, without downloading adware.
XMind Plus helps organize all details/aspects of any project from large to small in an orderly and concise manner. If you are working on a complex project, use this micro tutorial to show you how to make a basic flow chart. The software is free when…

728 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

11 Experts available now in Live!

Get 1:1 Help Now