Optimising if-else statements

[dev_15]

HI, Lookin at this code of mine i feel it could be better structured
as i seem to be checking
for Hi-res display twice. First i check for a square display, then
check for high res and load the appropriate image. If its not a square
display i again check for hi-res and load the appropriate images. Can
i reduce the number of if-else statements here or reorder it better
somehow.

if ( SQUARE == ortn ) {
if ( g_HIDPI_LogPixelsX >= 192 ) {
SetSquareLogo( IDR_SQUARE_HI );
}
else {
SetSquareLogo( IDR_SQUARE );
}
}

else {
if ( g_HIDPI_LogPixelsX >= 192 ) {

SetPortraitLogo( IDR_PORTRAIT_HI );
SetLandscapeLogo( IDR_LAND_HI );

}
else {

SetPortraitLogo( IDR_PORTRAIT );
SetLandscapeLogo( IDR_LAND );

}
}

 

[tragomaskhalos]

HI, Lookin at this code of mine i feel it could be better structured
as i seem to be checking
for Hi-res display twice. First i check for a square display, then
check for high res and load the appropriate image. If its not a square
display i again check for hi-res and load the appropriate images. Can
i reduce the number of if-else statements here or reorder it better
somehow.

if ( SQUARE == ortn ) {
if ( g_HIDPI_LogPixelsX >= 192 ) {
SetSquareLogo( IDR_SQUARE_HI );
}
else {
SetSquareLogo( IDR_SQUARE );
}
}

else {
if ( g_HIDPI_LogPixelsX >= 192 ) {

SetPortraitLogo( IDR_PORTRAIT_HI );
SetLandscapeLogo( IDR_LAND_HI );

}
else {

SetPortraitLogo( IDR_PORTRAIT );
SetLandscapeLogo( IDR_LAND );

}
}

bool fHi = (g_HIDPI_LogPixelsX >= 192);
if ( SQUARE == ortn ) {
SetSquareLogo( fHi ? IDR_SQUARE_HI : IDR_SQUARE);
} else {
SetPortraitLogo( fHi ? IDR_PORTRAIT_HI : IDR_PORTRAIT );
SetLandscapeLogo( fHi ? IDR_LAND_HI : IDR_LAND);
}

 

[Daniel T.]

HI, Lookin at this code of mine i feel it could be better structured
as i seem to be checking
for Hi-res display twice. First i check for a square display, then
check for high res and load the appropriate image. If its not a square
display i again check for hi-res and load the appropriate images. Can
i reduce the number of if-else statements here or reorder it better
somehow.

if ( SQUARE == ortn ) {
if ( g_HIDPI_LogPixelsX >= 192 ) {
SetSquareLogo( IDR_SQUARE_HI );
}
else {
SetSquareLogo( IDR_SQUARE );
}
}

else {
if ( g_HIDPI_LogPixelsX >= 192 ) {

SetPortraitLogo( IDR_PORTRAIT_HI );
SetLandscapeLogo( IDR_LAND_HI );

}
else {

SetPortraitLogo( IDR_PORTRAIT );
SetLandscapeLogo( IDR_LAND );

}
}

Not really. You have four different choices, so 3 if statements make
sense. Note, Tragomaskhalos' sample has *4* if statements in it, but
three of them are disguised using the ?: operator.

 

[Victor Bazarov]

Not really. You have four different choices, so 3 if statements make
sense. Note, Tragomaskhalos' sample has *4* if statements in it, but
three of them are disguised using the ?: operator.

There is a possibility to improve this using lookup tables:

static const unsigned P[] = { IDR_PORTRAIT, IDR_PORTRAIT_HI };
static const unsigned L[] = { IDR_LAND, IDR_LANG_HI };
static const unsigned S[] = { IDR_SQUARE, IDR_SQUARE_HI };

int high = g_HIDPI_LogPixelsX >= 192;

if ( SQUARE == ortn )
SetSquareLogo( S[high] );
else {
SetPortraitLogo( P[high] );
SetLandscapeLogo( L[high] );
}

I am not sure, however, that this is better than the original version.

V

 

[tragomaskhalos]

Not really. You have four different choices, so 3 if statements make
sense. Note, Tragomaskhalos' sample has *4* if statements in it, but
three of them are disguised using the ?: operator.- Hide quoted text -

True enough; I'd assumed the OP's question was motivated
by aesthetics rather than a desire to get the comparison
count down per se, and I reckon my version is clearer
(just!) than the original.

 

[Andrew Koenig]

Can i reduce the number of if-else statements here or reorder it better
somehow.

if ( SQUARE == ortn ) {
if ( g_HIDPI_LogPixelsX >= 192 ) {
SetSquareLogo( IDR_SQUARE_HI );
}
else {
SetSquareLogo( IDR_SQUARE );
}
}

else {
if ( g_HIDPI_LogPixelsX >= 192 ) {

SetPortraitLogo( IDR_PORTRAIT_HI );
SetLandscapeLogo( IDR_LAND_HI );

}
else {

SetPortraitLogo( IDR_PORTRAIT );
SetLandscapeLogo( IDR_LAND );

}
}

How about this? Yes, I know there are redundant parentheses, but they make
the precedence crystal clear.

The code is bigger, but it doesn't duplicate anything, and generalizes
easily to more than two conditions.

switch ((SQUARE == ortn) | ((g_HIDPI_LogPixelsX >= 192) << 1)) {

case false | (false<<1):
SetPortraitLogo( IDR_PORTRAIT );
SetLandscapeLogo( IDR_LAND );
break;

case false | (true<<1):
SetPortraitLogo( IDR_PORTRAIT_HI );
SetLandscapeLogo( IDR_LAND_HI );
break;

case true | (false<<1):
SetSquareLogo( IDR_SQUARE );
break;

case true | (true<<1):
SetSquareLogo( IDR_SQUARE_HI );
break;

default:
assert(false);
}

 

[Alf P. Steinbach]

* Andrew Koenig:

How about this? Yes, I know there are redundant parentheses, but they make
the precedence crystal clear.

The code is bigger, but it doesn't duplicate anything, and generalizes
easily to more than two conditions.

switch ((SQUARE == ortn) | ((g_HIDPI_LogPixelsX >= 192) << 1)) {

case false | (false<<1):
SetPortraitLogo( IDR_PORTRAIT );
SetLandscapeLogo( IDR_LAND );
break;

case false | (true<<1):
SetPortraitLogo( IDR_PORTRAIT_HI );
SetLandscapeLogo( IDR_LAND_HI );
break;

case true | (false<<1):
SetSquareLogo( IDR_SQUARE );
break;

case true | (true<<1):
SetSquareLogo( IDR_SQUARE_HI );
break;

default:
assert(false);
}

Uh, gurgle.

It simply implements a lookup table as code.

What's wrong with expressing the table as a table (using a helper
function SetNonSquareLogo it would be extremely simple)?


Wondering,

- Alf

 

[Michael Oswald]

How about this? Yes, I know there are redundant parentheses, but they
make the precedence crystal clear.

The code is bigger, but it doesn't duplicate anything, and generalizes
easily to more than two conditions.

switch ((SQUARE == ortn) | ((g_HIDPI_LogPixelsX >= 192) << 1)) {

case false | (false<<1):
SetPortraitLogo( IDR_PORTRAIT );
SetLandscapeLogo( IDR_LAND );
break;

case false | (true<<1):
SetPortraitLogo( IDR_PORTRAIT_HI );
SetLandscapeLogo( IDR_LAND_HI );
break;

case true | (false<<1):
SetSquareLogo( IDR_SQUARE );
break;

case true | (true<<1):
SetSquareLogo( IDR_SQUARE_HI );
break;

default:
assert(false);
}

Wow, really cool. Never saw this before...


lg,
Michael

 

[Daniel T.]

Wow, really cool. Never saw this before...

I started to post almost the exact same thing. I don't see where
replacing three if statements with 3 case statements (the above could be
reduced down to three case statements with no loss of expressiveness*)
is helpful though.

* remove the last case statement and make the default:

default:
assert( true | (true << 1) );
SetSquareLogo( IDR_SQUARE_HI );

 

[Andrew Koenig]

It simply implements a lookup table as code.

What's wrong with expressing the table as a table (using a helper function
SetNonSquareLogo it would be extremely simple)?

Nothing's wrong with it, but it's not as general. The original problem had
a choice among four alternatives, two of which happened to call
SetSquareLogo and the other two of which called SetPortraitLogo and
SetLandscapeLogo. The table approach captures that particular
characteristic of the choices in the structure of the tables.

By using a switch in the way I did, I made it possible for the choices to be
completely independent of each other in terms of what they did. This
independence came at the cost of a significant amount of extra code? Is
that right or wrong? Is it worth it? It's impossible to answer the
question in general. The purpose of my post was to illustrate a technique
that perhaps isn't so widely known, and to suggest evaluating it as a
possible answer in the context of its intended use.

I certainly wouldn't code that way all the time, but sometimes it might be
just the thing.

 

[Andrew Koenig]

I started to post almost the exact same thing. I don't see where
replacing three if statements with 3 case statements (the above could be
reduced down to three case statements with no loss of expressiveness*)
is helpful though.

In this particular example there's not much benefit. Where it might gain is
if the decisions in the code become more complicated.

Once upon a time I worked on a student-registration system that had sixteen
different procedures for computing tuition, depending on the student's
status. I don't remember the factors that went into the choice of
procedure, but they included things like whether the student was a graduate
student, whether the student was a state resident, and so on.