473,321 Members | 1,708 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,321 software developers and data experts.

Nested ifs and speed.

I recently came across s function (actually many) whose structure is a
series of nested ifs with the meat of the function at the very centre:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
return;
}
else {
printf("Condition3 failed");
return;
}
else {
printf("Condition2 failed");
return;
}
else {
printf("Condition1 failed");
return;
}

To me this is really unreadable and I would re-write as:

if(!condition1) {
printf("Condition 1 failed");
return;
}

....
<Body>
I got to thinking why would you write code this way; seperating the
condition from it's consequences. It occured to me that there may have
been performance related concerns with respect to jumps so with the
original code, in the case of all conditions suceeding, you get no
jumps but with my code you may get as many jumps as there are
conditions because each if block would need to be skipped. I am
certainly not an expert in compilers or machine code and this is
probably a case of premature optimization in either case I just trying
to get into the head of the guy that wrote it in the first place.

Cheers,
Charlie.

Jul 24 '06 #1
11 1406
"charlie" <ch*************@gmail.comwrote:
I recently came across s function (actually many) whose structure is a
series of nested ifs with the meat of the function at the very centre:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
return;
}
else {
printf("Condition3 failed");
return;
}
else {
printf("Condition2 failed");
return;
}
else {
printf("Condition1 failed");
return;
}

To me this is really unreadable and I would re-write as:

if(!condition1) {
printf("Condition 1 failed");
return;
}

...
<Body>

I got to thinking why would you write code this way; seperating the
condition from it's consequences. It occured to me that there may have
been performance related concerns with respect to jumps
Nah. It's probably just another Wirthian with blinkers on: One Entry,
One Exit, Or Else!

Richard
Jul 24 '06 #2

Richard Bos wrote:
>
Nah. It's probably just another Wirthian with blinkers on: One Entry,
One Exit, Or Else!

Richard
Actually, I just looked at the code again and saw that the else
branches do not contain return statement so you may well be correct:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
}
else {
printf("Condition3 failed");
}
else {
printf("Condition2 failed");
}
else {
printf("Condition1 failed");
}

Jul 24 '06 #3
charlie wrote:
>
Richard Bos wrote:
>>
Nah. It's probably just another Wirthian with blinkers on: One Entry,
One Exit, Or Else!

Richard

Actually, I just looked at the code again and saw that the else
branches do not contain return statement so you may well be correct:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
}
else {
printf("Condition3 failed");
}
else {
printf("Condition2 failed");
}
else {
printf("Condition1 failed");
}
But that code could be written with closer association as

if (!c1) printf( "blah");
else if (!c2) printf( "bleh" );
else if (!c3) printf( "blih" );
else if (!c4) printf( "bloh" );
else <body>

(layout and braces to taste), so the original layout doesn't
suggest having to stick to the one-exit style.

[Should there be a \n at the end of all these printfs?]

--
Chris "seeker" Dollin
"I'm still here and I'm holding the answers" - Karnataka, /Love and Affection/

Jul 24 '06 #4

"Chris Dollin" <ch**********@hp.comha scritto nel messaggio
news:ea**********@malatesta.hpl.hp.com...
charlie wrote:

Richard Bos wrote:
>
Nah. It's probably just another Wirthian with blinkers on: One Entry,
One Exit, Or Else!

Richard
Actually, I just looked at the code again and saw that the else
branches do not contain return statement so you may well be correct:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
}
else {
printf("Condition3 failed");
}
else {
printf("Condition2 failed");
}
else {
printf("Condition1 failed");
}

But that code could be written with closer association as

if (!c1) printf( "blah");
else if (!c2) printf( "bleh" );
else if (!c3) printf( "blih" );
else if (!c4) printf( "bloh" );
else <body>
Something like this ?

#define NC 2

static void (*condition[NC][NC][NC][NC])(void) = {
/* functions ... */
};
(*condition[!!(c1)][!!(c2)][!!(c3)][!!(c4)])();

or

(*condition[c1][c2][c3][c4])();

if c<i is 0 o 1.

I don't want to be time/space saving, just another idea.

Of course for a small number of conditions!
Observe that in this case ALL conditions are calculated!
--
Giorgio Silvestri



Jul 24 '06 #5
On 24 Jul 2006 07:01:15 -0700, "charlie" <ch*************@gmail.com>
wrote:
>I recently came across s function (actually many) whose structure is a
series of nested ifs with the meat of the function at the very centre:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
return;
}
else {
printf("Condition3 failed");
return;
}
else {
printf("Condition2 failed");
return;
}
else {
printf("Condition1 failed");
return;
}

To me this is really unreadable and I would re-write as:

if(!condition1) {
printf("Condition 1 failed");
return;
}

...
<Body>
I got to thinking why would you write code this way; seperating the
condition from it's consequences. It occured to me that there may have
been performance related concerns with respect to jumps so with the
original code, in the case of all conditions suceeding, you get no
jumps but with my code you may get as many jumps as there are
conditions because each if block would need to be skipped. I am
certainly not an expert in compilers or machine code and this is
probably a case of premature optimization in either case I just trying
to get into the head of the guy that wrote it in the first place.

Cheers,
Charlie.

One way to allocate and free some set of resources without retyping
the freeing code multiple times is nested ifs.

#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a, *b, *c;

a = malloc(1);
if(!a)
{
printf("allocation of a failed\n");
}
else
{
b = malloc(1);
if(!b)
{
printf("allocation of b failed\n");
}
else
{
c = malloc(1);
if(!c)
{
printf("allocation of c failed\n");
}
else
{
/* todo: do some operation which involves
* a, b, and c here */

free(c);
}

free(b);
}

free(a);
}

/* todo: set value based on success or failure */
return 0;
}
Another way is a goto with a cleanup label at the end of the function.
#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a = NULL, *b = NULL, *c = NULL;

a = malloc(1);
if(!a)
{
printf("allocation of a failed\n");
goto cleanup;
}

b = malloc(1);
if(!b)
{
printf("allocation of b failed\n");
goto cleanup;
}

c = malloc(1);
if(!c)
{
printf("allocation of c failed\n");
goto cleanup;
}

/* todo: do some operation which involves
* a, b, and c here */

cleanup:

if(a)
{
free(a);
}

if(b)
{
free(b);
}

if(c)
{
free(c);
}

/* todo: set value based on success or failure */
return 0;
}
Another way is a goto with a series of cleanup labels at the end of
the function, one for each resource.
#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a, *b, *c;

a = malloc(1);
if(!a)
{
printf("allocation of a failed\n");
goto cleanup_a;
}

b = malloc(1);
if(!b)
{
printf("allocation of b failed\n");
goto cleanup_b;
}

c = malloc(1);
if(!c)
{
printf("allocation of c failed\n");
goto cleanup_c;
}

/* todo: do some operation which involves
* a, b, and c here */

free(c);

cleanup_c:
free(b);

cleanup_b:
free(a);

cleanup_a:

/* todo: set value based on success or failure */
return 0;
}
If this is actually what you were asking about (?) then I think it's
only a style issue. No case should make much of a performance
difference. The middle one might be a few fractions of a percent
slower because of the extra ifs, but they should be quick compared to
the allocations. If the function really needed to be in a performance
critical path then it shouldn't even be doing the allocations anyway
(they should be done ahead of time somewhere else).

Jul 25 '06 #6
charlie wrote:
I recently came across s function (actually many) whose structure is a
series of nested ifs with the meat of the function at the very centre:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
return;
}
else {
printf("Condition3 failed");
return;
}
else {
printf("Condition2 failed");
return;
}
else {
printf("Condition1 failed");
return;
}

To me this is really unreadable and I would re-write as:

if(!condition1) {
printf("Condition 1 failed");
return;
}

...
<Body>
I got to thinking why would you write code this way; seperating the
condition from it's consequences. It occured to me that there may have
been performance related concerns with respect to jumps so with the
original code, in the case of all conditions suceeding, you get no
jumps
You have to jump at least once to get out from inside the body to
rejoin the main flow.
[...] but with my code you may get as many jumps as there are
conditions because each if block would need to be skipped. I am
certainly not an expert in compilers or machine code and this is
probably a case of premature optimization in either case I just trying
to get into the head of the guy that wrote it in the first place.
Well, if you have not determined that this is any important loop, then
it is a case of premature optimization. However, if you have
established that this is an important inner loop case, then it is not
premature. Very few programmers correctly comprehend the phrase
"premature optimization is the root of all evil". In particular most
cannot understand when the condition of "premature" has been
invalidated.

In any event, for clarity, there are two main categories for what is
going on here -- your inner loop and your errors. For clarity reasons,
this is how I would do it:

if (condition1 && condition2 && condition3 && condition4) {
/* No errors */
<Body>
} else {
/* Errors */
if (!condition1) printf("Condition1 failed");
/* else */ if (!condition2) printf("Condition2 failed");
/* else */ if (!condition3) printf("Condition3 failed");
/* else */ if (!condition4) printf("Condition4 failed");
return;
}

This is probably a lot easier to maintain.

For performance, there may be a question of accelerating: (condition1
&& condition2 && condition3 && condition4). If you expect all the
conditions to be true and equal to the value 1 (or have some shared
bit) most of the time, then you can change the && to a &, and this may
decrease the number instructions until you reach the body.

If you expect some of the conditions to fail, then leave the &&
operation, but the *order* in which you evaluate them will matter.
I.e., you want the condition to fail as quickly as possible, since the
order won't matter on success. So you put the conditions in decreasing
order of probability of failure. OTOH, the likelihood that this will
matter is basically 0, since you are eating penality for executing
"printf"'s on failure anyways.

--
Paul Hsieh
http://www.pobox.com/~qed/
http://bstring.sf.net/

Jul 25 '06 #7
On 24 Jul 2006 22:00:22 -0700, we******@gmail.com wrote:
>charlie wrote:
>I recently came across s function (actually many) whose structure is a
series of nested ifs with the meat of the function at the very centre:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
return;
}
else {
printf("Condition3 failed");
return;
}
else {
printf("Condition2 failed");
return;
}
else {
printf("Condition1 failed");
return;
}

To me this is really unreadable and I would re-write as:

if(!condition1) {
printf("Condition 1 failed");
return;
}

...
<Body>
I got to thinking why would you write code this way; seperating the
condition from it's consequences. It occured to me that there may have
been performance related concerns with respect to jumps so with the
original code, in the case of all conditions suceeding, you get no
jumps

You have to jump at least once to get out from inside the body to
rejoin the main flow.
>[...] but with my code you may get as many jumps as there are
conditions because each if block would need to be skipped. I am
certainly not an expert in compilers or machine code and this is
probably a case of premature optimization in either case I just trying
to get into the head of the guy that wrote it in the first place.

Well, if you have not determined that this is any important loop, then
it is a case of premature optimization. However, if you have
established that this is an important inner loop case, then it is not
premature. Very few programmers correctly comprehend the phrase
"premature optimization is the root of all evil". In particular most
cannot understand when the condition of "premature" has been
invalidated.

In any event, for clarity, there are two main categories for what is
going on here -- your inner loop and your errors. For clarity reasons,
this is how I would do it:

if (condition1 && condition2 && condition3 && condition4) {
/* No errors */
<Body>
} else {
/* Errors */
if (!condition1) printf("Condition1 failed");
/* else */ if (!condition2) printf("Condition2 failed");
/* else */ if (!condition3) printf("Condition3 failed");
/* else */ if (!condition4) printf("Condition4 failed");
return;
}

This is probably a lot easier to maintain.

That's not realistic. Each of those conditions is likely to be
calculated from the result of some function and also to have
calculations interspersed between it and the next condition, which
means not fitting nicely (or at all) into a single if().

Jul 25 '06 #8

BubbaGump wrote:
On 24 Jul 2006 07:01:15 -0700, "charlie" <ch*************@gmail.com>
wrote:
I recently came across s function (actually many) whose structure is a
series of nested ifs with the meat of the function at the very centre:

if (condition1)
if (condition2)
if (condition3)
if (condition4)
<Body>
else {
printf("Condition4 failed");
return;
}
else {
printf("Condition3 failed");
return;
}
else {
printf("Condition2 failed");
return;
}
else {
printf("Condition1 failed");
return;
}

To me this is really unreadable and I would re-write as:

if(!condition1) {
printf("Condition 1 failed");
return;
}

...
<Body>
I got to thinking why would you write code this way; seperating the
condition from it's consequences. It occured to me that there may have
been performance related concerns with respect to jumps so with the
original code, in the case of all conditions suceeding, you get no
jumps but with my code you may get as many jumps as there are
conditions because each if block would need to be skipped. I am
certainly not an expert in compilers or machine code and this is
probably a case of premature optimization in either case I just trying
to get into the head of the guy that wrote it in the first place.

Cheers,
Charlie.


One way to allocate and free some set of resources without retyping
the freeing code multiple times is nested ifs.

#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a, *b, *c;

a = malloc(1);
if(!a)
{
printf("allocation of a failed\n");
}
else
{
b = malloc(1);
if(!b)
{
printf("allocation of b failed\n");
}
else
{
c = malloc(1);
if(!c)
{
printf("allocation of c failed\n");
}
else
{
/* todo: do some operation which involves
* a, b, and c here */

free(c);
}

free(b);
}

free(a);
}

/* todo: set value based on success or failure */
return 0;
}
Another way is a goto with a cleanup label at the end of the function.
#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a = NULL, *b = NULL, *c = NULL;

a = malloc(1);
if(!a)
{
printf("allocation of a failed\n");
goto cleanup;
}

b = malloc(1);
if(!b)
{
printf("allocation of b failed\n");
goto cleanup;
}

c = malloc(1);
if(!c)
{
printf("allocation of c failed\n");
goto cleanup;
}

/* todo: do some operation which involves
* a, b, and c here */

cleanup:

if(a)
{
free(a);
}

if(b)
{
free(b);
}

if(c)
{
free(c);
}

/* todo: set value based on success or failure */
return 0;
}
Another way is a goto with a series of cleanup labels at the end of
the function, one for each resource.
#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a, *b, *c;

a = malloc(1);
if(!a)
{
printf("allocation of a failed\n");
goto cleanup_a;
}

b = malloc(1);
if(!b)
{
printf("allocation of b failed\n");
goto cleanup_b;
}

c = malloc(1);
if(!c)
{
printf("allocation of c failed\n");
goto cleanup_c;
}

/* todo: do some operation which involves
* a, b, and c here */

free(c);

cleanup_c:
free(b);

cleanup_b:
free(a);

cleanup_a:

/* todo: set value based on success or failure */
return 0;
}
If this is actually what you were asking about (?) then I think it's
only a style issue. No case should make much of a performance
difference. The middle one might be a few fractions of a percent
slower because of the extra ifs, but they should be quick compared to
the allocations. If the function really needed to be in a performance
critical path then it shouldn't even be doing the allocations anyway
(they should be done ahead of time somewhere else).
It is actually not mallocs but fopens but the principle is pretty much
the same. I have looked at more of the code now and I've come to the
conclusion that readability was not a priority. There are several >
1000 line procedures and nearly no comments. Thank-you (all) for your
suggestions on how to make it more readable and sensible. I will put
them into practice when I have introduced the code to the concept of
abstraction (and other coding practices too). I just want to be able to
read and debug the code without feeling like I need to delete the whole
shebang and start again, which will never happen of course.
Frustrated coders of the world unite!

Cheers
Charlie.

Jul 25 '06 #9
charlie <ch*************@gmail.comwrote:
I recently came across s function (actually many) whose structure is a
series of nested ifs with the meat of the function at the very centre:
(code snipped)
To me this is really unreadable and I would re-write as:
if(!condition1) {
printf("Condition 1 failed");
return;
}
...
<Body>
Alternately, you could abuse the preprocessor a bit:

#define VALIDATE(c) (c?1:(printf("Condition "#c" failed\n"),0))

if( VALIDATE(condition1) && VALIDATE(condition2) && ... ) {
<body>
}

(I make no representations about whether this would be advisable.
Notice that you end up with an additional conditional check for every
condition, which may not always be neglible.)

--
C. Benson Manica | I *should* know what I'm talking about - if I
cbmanica(at)gmail.com | don't, I need to know. Flames welcome.
Jul 25 '06 #10

In article <fm********************************@4ax.com>, BubbaGump writes:
>
One way to allocate and free some set of resources without retyping
the freeing code multiple times is nested ifs. [...]

Another way is a goto with a cleanup label at the end of the function.

#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a = NULL, *b = NULL, *c = NULL;

[...]

cleanup:

if(a)
{
free(a);
}

if(b)
{
free(b);
}

if(c)
{
free(c);
}
free(NULL) is guaranteed safe. You can replace this fragment with:

cleanup:
free(a);
free(b);
free(c);

which takes far less vertical space, allowing the reader to see more
of the surrounding function.

Of course, the same is not true of many other cleanup functions,
such as fclose (for some inexplicable reason). For the specific
case of cleanup code at the end of a function, though, I personally
don't mind an abbreviated style, as in:

cleanup:
free(a);
if (b) fclose(b);
free(c);

which ought to be perfectly clear to the reader - assuming that
the "cleanup" section of code in fact just performs cleanup.

--
Michael Wojcik mi************@microfocus.com

Even though there may be some misguided critics of what we're trying
to do, I think we're on the wrong path. -- Reagan
Jul 25 '06 #11
On 25 Jul 2006 16:51:03 GMT, mw*****@newsguy.com (Michael Wojcik)
wrote:
>
In article <fm********************************@4ax.com>, BubbaGump writes:
>>
One way to allocate and free some set of resources without retyping
the freeing code multiple times is nested ifs. [...]

Another way is a goto with a cleanup label at the end of the function.

#include <stdio.h>
#include <stdlib.h>

int main()
{
void *a = NULL, *b = NULL, *c = NULL;

[...]

cleanup:

if(a)
{
free(a);
}

if(b)
{
free(b);
}

if(c)
{
free(c);
}

free(NULL) is guaranteed safe. You can replace this fragment with:

cleanup:
free(a);
free(b);
free(c);

which takes far less vertical space, allowing the reader to see more
of the surrounding function.

Of course, the same is not true of many other cleanup functions,
such as fclose (for some inexplicable reason). For the specific
case of cleanup code at the end of a function, though, I personally
don't mind an abbreviated style, as in:

cleanup:
free(a);
if (b) fclose(b);
free(c);

which ought to be perfectly clear to the reader - assuming that
the "cleanup" section of code in fact just performs cleanup.
I like it.

Jul 26 '06 #12

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

0
by: Francis Avila | last post by:
A few days ago (see the 'itertools.flatten()?' thread from October 28) I became obsessed with refactoring a recursive generator that yielded the leaves of nested iterables. When the dust settled,...
25
by: chad | last post by:
I am writing a program to do some reliability calculations that require several nested for-loops. However, I believe that as the models become more complex, the number of required for-loops will...
3
by: Eirik Eldorsen | last post by:
Im trying to make a nested repeater with 3 levels. I've successfully created a nested repeater with 2 levels, but when adding the 3rd level I get an InvalidCastException. What am I doing wrong? ...
77
by: Peter Olcott | last post by:
http://www.tommti-systems.de/go.html?http://www.tommti-systems.de/main-Dateien/reviews/languages/benchmarks.html The above link shows that C# is 450% slower on something as simple as a nested loop....
2
by: mavrick_101 | last post by:
Hi, I have a nested repeater. For each row in the parent repeater there is a child repeater with several rows of data. The way I'm doing this is throught ItemDataBound Event. So for each Row...
5
by: request | last post by:
I have a little piece of code that compiles fine but I think it shouldn't compile fine. Here it is: class outer_class { public: outer_class () {} int operator () (int i1, int i2) { return...
4
by: =?Utf-8?B?Qnlyb24=?= | last post by:
When I try to serialize an instance of the LocationCell below (note Building field) I get an error in the reflection attempt. If I remove the _Building field it serializes fine. I tried renaming...
2
by: ASF | last post by:
Hey all, I have a gridview which pulls from a BLL which pulls from a DAL (an .XSD file). Each row on that gridview has a nested repeater which pulls from another table. The code which populates...
0
by: DolphinDB | last post by:
Tired of spending countless mintues downsampling your data? Look no further! In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
0
by: ryjfgjl | last post by:
ExcelToDatabase: batch import excel into database automatically...
0
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: Vimpel783 | last post by:
Hello! Guys, I found this code on the Internet, but I need to modify it a little. It works well, the problem is this: Data is sent from only one cell, in this case B5, but it is necessary that data...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
1
by: PapaRatzi | last post by:
Hello, I am teaching myself MS Access forms design and Visual Basic. I've created a table to capture a list of Top 30 singles and forms to capture new entries. The final step is a form (unbound)...
1
by: Shællîpôpï 09 | last post by:
If u are using a keypad phone, how do u turn on JavaScript, to access features like WhatsApp, Facebook, Instagram....
0
by: Faith0G | last post by:
I am starting a new it consulting business and it's been a while since I setup a new website. Is wordpress still the best web based software for hosting a 5 page website? The webpages will be...
0
isladogs
by: isladogs | last post by:
The next Access Europe User Group meeting will be on Wednesday 3 Apr 2024 starting at 18:00 UK time (6PM UTC+1) and finishing by 19:30 (7.30PM). In this session, we are pleased to welcome former...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.