Thread: if() block versus ? : cond operator

Ok, like I promised in another message, I will ask some questions

Here's the if code which only checks iFooOK[] array index 0 and 1 and set's variable iBar accordingly to index so if index 0 equals BAR_OK then I get index 0, else if index 1 equals BAR_OK then I get 1, if both are not _OK then -1.

Code:

if( BAR_OK == iFooOK[ 0 ] )
{
	iBar = 0;
}
elseif( BAR_OK == iFooOK[ 1 ] )
{
	iBar = 1;
}
else
{
	iBar = -1;
}

I thought that's taking little too much space and squeezing space "misplacing" block marks didnt serve my appetite, I thought to try out " ? : "

So, here's the result.

Code:

iBar = (iFooOK[ 0 ] == BAR_OK ? iBar = 0 :
	(iFooOK[ 1 ] == BAR_OK ? iBar = 1 : -1));

The fact is, readability of this shorter version is close to zero (though, I can write one line of comment to fully describe the intent of this piece of code)
So, I am again asking pro's to nudge me into right direction if there's a better way for this particular case.
Especially if in the far future I find myself in situation where there is not only 2 items in this array ("How could it be? I specified it's going to be only 2 forever and all of sudden there needs to be 3!!").
In that case, should I just #define array size:

Code:

#define MAX_BARS 3
iBar = -1;

for( i = 0; i < MAX_BARS; i++ )
{
	if( BAR_OK == iFooOK[i] )
	{
		iBar = i;
		break;
	}
}

Now, ok.. for() aint that cool compared to ? : conditional operator. But at this point, I am thinking to just trash this post and move on with my coding and use for(). Everything is bound to change in future and are bound to break boundaries.

But I post this anyway (to increase my post count... NONO! I mean, to show an example.)
And a hope of an opinion

How I would write that code...

Code:

if( BAR_OK == iFooOK[ 0 ] ) {
	iBar = 0;
} else if( BAR_OK == iFooOK[ 1 ] ) {
	iBar = 1;
} else {
	iBar = -1;
}

And in general the ternary operator isn't used for the reason you have found, it is a pain to read heh. Using defines is a way to do things, but better would be to just use a const int at global scope. Using defines takes away the chance for good type checking.

>I thought that's taking little too much space
Shorter code is not always better code. However, you can generalize the block if you intend to add more iFooOk's and continue to follow the pattern of iBar being set to the index:

Code:

#define N_OK 2

int i;

for ( i = 0; i < N_OK; i++ ) {
  if ( iFooOK[ i ] == BAR_OK )
    break;
}

iBar = ( i != 3 ) ? i : -1;

Rather than just "making it shorter", that's a legitimate improvement to maintainability, and it's also shorter. The conditional operator is fine as long as you keep it simple. Nesting them is almost always a bad idea.

Not only do you know it is an int and nothing else, so does the compiler!

Originally Posted by Prelude

>I thought that's taking little too much space
Shorter code is not always better code. However, you can generalize the block if you intend to add more iFooOk's and continue to follow the pattern of iBar being set to the index:

Code:

#define N_OK 2

int i;

for ( i = 0; i < N_OK; i++ ) {
  if ( iFooOK[ i ] == BAR_OK )
    break;
}

iBar = ( i != 3 ) ? i : -1;

Rather than just "making it shorter", that's a legitimate improvement to maintainability, and it's also shorter. The conditional operator is fine as long as you keep it simple. Nesting them is almost always a bad idea.

What's the point of the three? i will never be three. Here the loop stops on "i == N_OK" which should be 2. So unless I'm mistaken, it should be:

Code:

ibar = ( i != N_OK ) ? i : -1;

If not, it should be:

Code:

ibar = ( i != N_OK + 1 ) ? i : -1;

but I don't think so.


Quzah.

>What's the point of the three?
It's a feature. I originally used 3 directly as the size of the array and forgot to change all occurances when I switched to a macro.

>So unless I'm mistaken
You're not, I was. My logic was sound until I tried to clean up the example. Go figure.

I figured as much, but it's not often I get to correct you, so I had to make the best of it.
Plus, since it was you and not some random peon, I actually had to stop and think about it, in case you were doing something sneaky.


Quzah.

I don't like the style of putting braces on the end of the starting line of a block. Ick. Just personal preference but for me it's easier to distinguish the blocks if the braces for them are given a sep line.

I only use the ternary operator on very simple if conditions. However I've noticed that C++ test creators love to use (and abuse) this feature as well as completely obfuscate a simple if() block.

What does this code mean:

Code:

if ( ( ( (a & b) & c) & 0xFF) || 1==((c & d) | (a & b)) ) ....

A. 1
B. 2
C. 3
D. Good example of how to abuse an if block

I also don't like my 'else' on the same line as the 'if', but that's just me.

Code:

if( x )
{
    ...
}
else
if( y )
{
    ...
}
else
{
    ...
}

Quzah.

Originally Posted by quzah

I figured as much, but it's not often I get to correct you, so I had to make the best of it.
Plus, since it was you and not some random peon, I actually had to stop and think about it, in case you were doing something sneaky.


Quzah.

Stop, you're making me blush.

>I don't like the style of putting braces on the end of the starting line of a block. Ick.
Neither do I. But I don't like the other styles any better, and I've already formed a habit. You know, in theory, the most readable style is Whitesmith:

Code:

#include <stdio.h>

int main ( void )
  {
  int i;

  for ( i = 0; i < 10; i++ )
    {
    if (i % 2 == 0)
      {
      printf ( "%d\n", i );
      }
    }

  return 0;
  }

This must be one of those times where readability and aesthetics don't go hand in hand. I think Whitesmith is ugly as sin.

Originally Posted by Prelude

Stop, you're making me blush.

This must be one of those times where readability and aesthetics don't go hand in hand. I think Whitesmith is ugly as sin.

Agreed....

I've got a bastardized style that I use: I'll put the brace on the end of the beginning line of an if, for, while, switch (etc) statement. But when I'm doing function blocks, the opening brace is always on a separate line.

Works for me, and I'm always consistent....

And regarding the original question, I will use the ternary operator in very simple cases, where its use is pretty self-evident. Otherwise, it's good old if-else.