Optimization of if

[Rajen]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}


The above if can be written like this
int retVal = 0;
if(X)
{
retVal=retVal1;
}

if( retVal == 0)
{
if( Y)
retVal=retVal2;
}

if( retVal == 0 )
{
if(Z)
retVal=retVal3;
}

return retVal;
which has only one exit point. But it has more If's

How do i Optimize this? Please give me some suggestions.

 

[Chris Dollin]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}


The above if can be written like this
int retVal = 0;
if(X)
{
retVal=retVal1;
}

if( retVal == 0)
{
if( Y)
retVal=retVal2;
}

if( retVal == 0 )
{
if(Z)
retVal=retVal3;
}

return retVal;
which has only one exit point. But it has more If's

How do i Optimize this? Please give me some suggestions.

Optimise? What's to optimise? I didn't see anything non-optimal
with the first one. (There's a missing case for when none of
X, Y, and Z are true.) The second form seems to assume that
neither `retVal1` nor `retVal2` are 0. Maybe that's true but
it seems to be asking for trouble to rely on it. And that second
form is clumsier and harder to understand.

Myself I prefer the form:

return
X ? retVal1
: Y ? retVal2
: Z ? retVal3
: whateverYouWantForNoneOfXYZ
;

laid out in whatever way fits your coding style.

Optimisation? This is really the bottleneck in your code? Colour
me gobsmacked.

 

[Richard Tobin]

I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}


The above if can be written like this
int retVal = 0;
if(X)
{
retVal=retVal1;
}

if( retVal == 0)
{
if( Y)
retVal=retVal2;
}

if( retVal == 0 )
{
if(Z)
retVal=retVal3;
}

return retVal;
which has only one exit point. But it has more If's

Your first form is much clearer, so use that. If your employer insists on
less clear code to satisfy some rules, find another employer.

-- Richard

 

[Laurent Deniau]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}

which has only one exit point. But it has more If's

How do i Optimize this? Please give me some suggestions.

Nothing says the first case generate more than one exit point of your
function. BTW some compilers will effectively generate only one exit
point and jump to it from each return point, so it far from obvious that
first case will not be optimal. Clarity is much more important than
implementation dependant low-level assumption.

a+, ld.

 

[Eric Sosman]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}


The above if can be written like this
int retVal = 0;
if(X)
{
retVal=retVal1;
}

if( retVal == 0)
{
if( Y)
retVal=retVal2;
}

if( retVal == 0 )
{
if(Z)
retVal=retVal3;
}

return retVal;
which has only one exit point. But it has more If's

How do i Optimize this? Please give me some suggestions.

First, it's extremely unlikely that any optimization
is needed.

Second, learn about the `else' keyword.

 

[Chris Dollin]

Nothing says the first case generate more than one exit point of your
function.

There are three exit points show: each return is an exit point.
(It doesn't matter how the compiler compiles it [1]; I suspect
the OP is afflicted by Code Style Rules that values Conformance
over Clarity.)

[1] EG the compiler [2] might translate `return 0;`, `return 1;`
and `return -1;` to jumps into the implementation library
code to load the constant and then return. One might then
say there were /no/ exit points in a function with those
return values ...

[2] OK, it was an RTL/2 compiler, not a C compiler, optimising
for space.

 

[Richard Bos]

if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}

The above if can be written like this
int retVal = 0;
if(X)
{
retVal=retVal1;
}

if( retVal == 0)
{
if( Y)
retVal=retVal2;
}

if( retVal == 0 )
{
if(Z)
retVal=retVal3;
}

return retVal;
which has only one exit point. But it has more If's

How do i Optimize this? Please give me some suggestions.

As others have noted, you're missing a default value.

Try this:

retval=default_value;
if (Z) retval=retval3;
if (Y) retval=retval2;
if (X) retval=retval1;
return retval;

Then immediately return to the first, maintainable version, but lay it
out a bit more legibly:

if(X) return retVal1;
if(Y) return retVal2;
if(Z) return retVal3;

or if you like braces:

if(X) { return retVal1; }
if(Y) { return retVal2; }
if(Z) { return retVal3; }

Richard

 

[David T. Ashley]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}


The above if can be written like this
int retVal = 0;
if(X)
{
retVal=retVal1;
}

if( retVal == 0)
{
if( Y)
retVal=retVal2;
}

if( retVal == 0 )
{
if(Z)
retVal=retVal3;
}

return retVal;
which has only one exit point. But it has more If's

You have three options for optimization.

a)First, note in your second example that this is a control flow problem.
Your problem is keeping the value of retVal from being multiply assigned.
It is more economical to do this with an else-if statement, i.e.

if (X)
retVal = retVal1;
else if (Y)
retVal = retVal2;
else
retVal = retVal3;

This suppresses the extra "0" tests and does what you want. Also, if your
possibilities are mutually exclusive and/or exhaustive, you can suppress the
final Z test.

b)Else-if has the limit that in order to get to clause N, all of the
preceding N tests must have been evaluated. If your problem is amenable to
it, try using the switch() statement.

c)Finally, in special cases where switch() can't be used, there are
sometimes relationships that let one optimize. For example, assume that I
want a fast implementation of floor(log_10(x)) up through 10,000. I could
do it like this:

if (x < 100)
{
if (x < 10)
{
}
else
{
}
}
else
{
if (x < 1000)
{
}
else
{
}
}

When the domain is "paneled", you can often build an if() construct where
the time to reach the innermost clause is related to log(N) where N is the
number of cases.

Notice that the construct above gets there in 2 tests (rather than up to
4).

 

[Default User]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}
[snip]

How do i Optimize this? Please give me some suggestions.

The more typical way to write this if you want to have only one return
statement is with an if-else if ladder.

int retVal = 0; /* or other default value */

if(X)
{
retVal = retVal1;
}
else if(Y)
{
retVal = retVal2;
}
else if(Z)
{
retVal = retVal3;
}

return retVal;

This method probably is about as efficient as the one with returns in
the if() statements, understanding that such things are up to the
implementation.

You could also skip the initialization of retVal and put a terminating
else that sets it to the "no hit" value, but I'd prefer the method
above. If the return value must be one of the three, then set retVal to
one of the values and use only two if()'s.




Brian

 

[Keith Thompson]

if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}

[...]
As others have noted, you're missing a default value.

Try this:

retval=default_value;
if (Z) retval=retval3;
if (Y) retval=retval2;
if (X) retval=retval1;
return retval;

That's likely to be (slightly) *less* efficient, since it can assign
to retval multiple times. It can also behave differently if X, Y, and
Z are expressions that depend on each other's evaluation; for example,
evaluating Y might not work if X hasn't already been evaluated.

Then immediately return to the first, maintainable version, but lay it
out a bit more legibly:

if(X) return retVal1;
if(Y) return retVal2;
if(Z) return retVal3;

or if you like braces:

if(X) { return retVal1; }
if(Y) { return retVal2; }
if(Z) { return retVal3; }

I'd probably lay it out that way if the conditions and result
expressions really were that terse, except that I always put a blank
after an "if" (it's not a function call, so it shouldn't look like
one):

if (X) return retVal1;
if (Y) return retVal2;
if (Z) return retVal3;

But in real life, everything is likely to be longer, perhaps too long
to fit on a single line. And if the conditions really were "X", "Y",
and "Z", I'd seriously consider using names that make some actual
sense.

 

[Keith Thompson]

Nothing says the first case generate more than one exit point of your
function.

There are three exit points show: each return is an exit point.
(It doesn't matter how the compiler compiles it [1]; I suspect
the OP is afflicted by Code Style Rules that values Conformance
over Clarity.)

Possibly, but we don't really know that. The question was "How do i
Optimize this?", not "How do I optimize this while conforming to
certain code style rules that I haven't bothered to tell you you
about?".

There are real advantages in restricting yourself to a single exit
point, but they show up more in maintenance than in performance. For
example, if you later find that you need to perform some action before
returning a value, given the above quoted code you need to do it in
three places. If you instead save the result to a single variable,
add "else"s, and return the value of the variable at the bottom, you
can add the extra code in just one place.

Whether this means you should *always* have just one exit point is a
subject of considerable debate.

 

[Richard Harter]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}


The above if can be written like this
int retVal = 0;
if(X)
{
retVal=retVal1;
}

if( retVal == 0)
{
if( Y)
retVal=retVal2;
}

if( retVal == 0 )
{
if(Z)
retVal=retVal3;
}

return retVal;
which has only one exit point. But it has more If's

How do i Optimize this? Please give me some suggestions.

I assume that you are asking how to optimize the clarity and
maintainability of the code rather than speed. There have been many
excellent answers. Here is a different kind of answer.

int retval_index = 0;
int retval[] = {retval_default, retval1, retval2, retval3};

if (X) retval_index = 1;
else if (Y) retval_index = 2;
else if (Z) retval_index = 3;

return retval[retval_index];

 

[CBFalconer]

Your first form is much clearer, so use that. If your employer
insists on less clear code to satisfy some rules, find another
employer.

No, just write clearer and more compact code:

int retval;

if (X) retval = retval1;
else if (Y) retval = retval2;
else if (Z) retval = retval3;
else retval = 0;
return retval;

 

[Christopher Layne]

That's likely to be (slightly) *less* efficient, since it can assign
to retval multiple times. It can also behave differently if X, Y, and
Z are expressions that depend on each other's evaluation; for example,
evaluating Y might not work if X hasn't already been evaluated.

Agreed. Although any compiler written within the last 15 years will just
optimize this out (provided optimization is turned on). The amount of time
even I've just spent typing this message is more time than the difference
would ever amount to in the long run.

 

[Christopher Layne]

There are three exit points show: each return is an exit point.
(It doesn't matter how the compiler compiles it [1]; I suspect
the OP is afflicted by Code Style Rules that values Conformance
over Clarity.)

Possibly, but we don't really know that. The question was "How do i
Optimize this?", not "How do I optimize this while conforming to
certain code style rules that I haven't bothered to tell you you
about?".

About the only way I can see optimizing this cascade is if the conditions X,
Y, Z had varying levels of computational demand. Obviously, the correct
optimization is to move the most common case to the top - provided the
secondary and tertiary cases are not exception to the common case. But we
don't know the OPs conditions.

e.g. (and I know this is obvious to you Keith):

if (compute_entire_disk_space_used() > 0)
return retVal2;
else if ((rand() + ((v & y) << 4)) % 78)
return retVal1;
else if (c > 0)
return retVal3;

I think it's obvious which changes should be made there.

 

[Christopher Layne]

Myself I prefer the form:

return
X ? retVal1
: Y ? retVal2
: Z ? retVal3
: whateverYouWantForNoneOfXYZ
;

laid out in whatever way fits your coding style.

Do you like pain?

 

[Richard]

No, just write clearer and more compact code:

int retval;

if (X) retval = retval1;
else if (Y) retval = retval2;
else if (Z) retval = retval3;
else retval = 0;
return retval;

In commercial SW, multiple statements/expressions on a line are neither
clearer nor more "compact". The layout above is horrible and totally
debugger unfriendly. The excessive whitespace breaks the flow of a code
read. the lack of indentation doesnt hilite the conditional
assignments. 0 out of 10.

 

[Christopher Layne]

In commercial SW, multiple statements/expressions on a line are neither
clearer nor more "compact". The layout above is horrible and totally
debugger unfriendly. The excessive whitespace breaks the flow of a code
read. the lack of indentation doesnt hilite the conditional
assignments. 0 out of 10.

Pretty common style idiom for extremely simple maintenance logic. Get used to
it. Obviously for a more complex case that is not filled with rudiment, you
will see multiple lines. I just think you're pissed off at the "style"
itself.

 

[Nishu]

Hi all,
I have a code like this.
if(X)
{
return retVal1;
}

if(Y)
{
return retVal2;
}

if(Z)
{
return retVal3;
}

How do i Optimize this? Please give me some suggestions.

Your first code is optimized in speed as well as size. What more
optimizations do you need?

-Nishu

 

[Nishu]

First, it's extremely unlikely that any optimization
is needed.

Second, learn about the `else' keyword.

Is 'else' keyword really necessary? What is the drawback in using only
if statements?

Thanks,
Nishu