Are multiple conditional operators in this situation a good idea?

Question

I just saw this block of code on the Wikipedia article on conditional operators:

Vehicle new_vehicle = arg == 'B' ? bus      :
                      arg == 'A' ? airplane :
                      arg == 'T' ? train    :
                      arg == 'C' ? car      :
                      arg == 'H' ? horse    :
                      feet;

I've changed the code a little, but the idea is the same. Would you find this use of the conditional operator acceptable? It's much more concise than the if-else construct, and using a switch would definitely open up a whole new set of opportunities for bugs (fall-throughs anyone?). Also, if-elses and switch can't be used as R-values, so you'd have to create the variable first, initialize it and then assign as necessary.

I for one really like this, but I'm wondering what others think.

But the formatting is essential.

EDIT: I still like this. But I understand those who say "the switch statement was made for this". OK, maybe so. But what if the conditions are function calls that return bool? Or a million other things you can't switch on.

Are you switch lovers really trying to convince me that a huge if-else chain is better? Yes, programmers who don't know how to use the conditional operator will not understand this. They should learn how to use it. It's not arcane.

Answer 1

I would use a switch, because this is the kind of thing it was designed for. Yes, there is a risk of fall-through bugs, but this nested-conditional block has a much higher risk of being misunderstood by other programmers.

Answer 2

I have used this type of construction many times. As long as it's formatted nicely (i.e. not all on one line, making it unreadable), I don't see a problem with it.

Answer 3

I like it. It is similar to an if-else-if ladder, only more concise.

Answer 4

This is a great example of conditional operator use. I use it in this manner all the time in C++, Java, and Perl.

Answer 5

A switch is both clearer and possibly much more efficient. If I saw code like this at a code review, I'd be worried. Also, this is "the conditional operator" - it is an instance (albeit currently the only one in C and C++) of a ternary operator.

Answer 6

Not only is there nothing wrong with it, it communicates the intent of the operation in the most concise and clear way possible.

Replacing with if else, or switch construction requires that the snippet

"new_vehicle =  "

be repeated in every instance, which requires that the reader read every repeating instance of it to ensure that it is in fact the same in every instance..

Answer 7

Purely a stylistic choice. For small data sets like you present here, then as long as your programming team isn't thrown by such a thing, then its fine in my book.

Answer 8

I've never seen anything written like this before. While it is clever and well-formatted, this seems like the perfect opportunity to use a dictionary/hashtable (assuming Vehicle is an enumeration, which is unclear).

Answer 9

Many of us are used to the Iif-functions in various reporting tools or the If-function in Excel where we basically need to use the less clear Iif(arg="B";"Bus";Iif(arg="A";Airplane;"Feet")). I love your sample compared to that :) I personally would've used if-elses, but I would not have a problem with your sample.

Answer 10

I would lean toward a switch statement because the compiler will catch duplicate cases. Arguably, this is not an issue in this example but if the list gets really long and is worked on by several different people it is easy to add a duplicate and not realize it.

Answer 11

The conditional operator version is clean, simple, and it's immediately obvious to anybody who knows C or C++ what is happening. Its other virtue is that it returns a value immediately, which means it can be put in the initialization (like this example is).

A switch statement would be more clumsy. It would require that the variable be declared and then initialized, usually a bad idea if it can be avoided. It would take more typing, and would have more places for bugs to creep in. It wouldn't be as clear, since it would be necessary to look at each case to see that it said something like new_vehicle = foo; break;.

If you're going to do this look up here only, then having the conditional version right there is good, as it immediately shows what's happening. If it's going to occur more than once, consider putting it in a function, so that there's only one place to update if anything changes (such as, say, 'R' for carriage or 'L' for helicopter).

Answer 12

Read the C++ section of the Wikipedia article a bit more carefully. It explicitly lists some situations where using the ?: operator is the only option, and can't be replaced by if/else or switch.

On the other hand, I wouldn't use it only because it looks prettier.

Answer 13

Vehicle new_vehicle = getVehicleByType(arg);

Vehicle getVehicleByType(char arg){
  if (arg == 'B') return bus;
  if (arg == 'A') return airplane;
  if (arg == 'C') return car;
  if (arg == 'T') return train;
  if (arg == 'H') return horse;
  return feet;
}

I like this better. The nested conditional is clever, but I think this is almost as concise and less likely to confuse a future reader. Sorry if the syntax is off, I'm not doing much C nowadays.

EDIT: Fixed return type omission noted in comments. thx!

EDIT: I'm not horrified at your version by the way. I did not exclaim WTF or OMG when i saw it. I just prefer mine a little more :)

Answer 14

A few people have already mentioned the possibility of using an std::map or other associative array type to do the job. As long as you're only doing this is one place (or a few places), you might consider just using a normal array or vector instead:

Vehicle vehicles[CHAR_MAX];

// Initialization    
std::fill_n(vehicles, CHAR_MAX, feet);
vehicles['A'] = airplane;
vehicles['B'] = bus;
vehicles['C'] = car;
vehicles['H'] = horse;
vehicles['T'] = train;

// Use
Vehicle new_vehicle = vehicles[arg];

Depending on how may tables you need/use (that store the same type of object), and the size of the contained objects (Vehicle in this case), this can be a perfectly reasonable alternative to an std::map. If you're creating a lot of tables, or each object is really big, std::map becomes a more reasonable alternative.

When you use an std::map (or unordered_map, etc.) you're using more code to save on data storage. This does the reverse -- but as long as Vehicle is small (say, 4 bytes), one table like the above will typically occupy something like half a kilobyte. It's hard to guess exactly how large the code for std::map is going to be for a specific compiler, but it seems likely that it'll usually be larger than half a kilobyte, so if you're only creating one table like this, std::map may be a net loss.

Of course, if you know that you're only dealing with letters as input, you could reduce the table size quite a bit:

template <class T>
class letter_table { 
    static const int range = 'Z' - 'A';

    T table[range];
public:
    // ...
    T operator[](int index) {
        index -= 'A';
        assert(index<range);
        return table[index];
    }
};

In the example case, this would give a table around 100 bytes -- you can create a pretty fair number of 100-byte tables in the space std::map will normally occupy.

Answer 15

Purely practical:

Plus: The ternary sequence is more flexible, and can be used to avoid the limitations of switch, you could use other operators (e.g. <=, >=) or any other tests, including e.g. string comparisons.

x = IsEven(arg) ?  0 : 
    (arg < 0)   ? -1 : 1; // or whatever

Also, if the switch is a performance bottleneck and you have uneven probabilities, you can force the likeliest tests being done first (due to the not evaluated guarantee for the path not chosen).

So-So Unlike a switch statement, order is important (unless you stick to ==). That can be an advantage, but being otherwise similar to switch that might be misleading when the maintainer is unfamiliar with the concept or in a hurry.

Many developers may shy away because they aren't sure about the details (which terms will be evaluated, is the rpecedence of the operators ok?) - However, if your developer pool won't grasp a well-presented example, you might have problems that can't be solved by banning ternary operators.

Minus It isn't as common as switch, thus the optimizer might not treat it the same. Optimizers are know to select the best fit implementation for a switch (table, binary search, comparison sequence, or any combination of this). Optimizers can't reaarange wevaluaiton order, and are less likely to support a table lookup here.

Requires good formatting to be easily recognizable (lining up the '?' and ':') - Sucks when you use tabs.

Aesthetics

I like it for its precision and succinctness, close to mathematical notations. However, that might as well be used against it. It's probably an eyebrow raiser at code reviews, because it is less common and more brittle.

Answer 16

Just for comparison, in C++0x you can have an expression without using the conditional operator or an out-of-line function:

Vehicle new_vehicle = [&]() -> Vehicle {
    if (arg == 'B') return bus;
    if (arg == 'A') return airplane;
    if (arg == 'T') return train;
    if (arg == 'C') return car;
    if (arg == 'H') return horse;
    return feet;
}();

Not really any better, though.

Answer 17

In my opinion what you've done is acceptable due to the simplicity of the example. If you were doing more things with each case this type of construct could rapidly get messy. For that reason I would prefer a switch or even nested if then elses (if there aren't too many cases), formatted as follows:

  if (A) {
      //Do A stuff
  }
  else if (B) {
      //Do B stuff
  }
  else if (C) {
      //Do C stuff
  }
  else {
      //Do default stuff
  }

It's about the readability of the code, which lends itself to the maintainability of the code. I've never been a huge fan fan of the conditional operator, because I don't like to see multiple expressions on a single line. Conditional operators can be difficult to follow when single stepping the code in a debugger. The simpler the code the easier it is to concentrate on what the code is doing.

Answer 18

There's a lot of whitespace around the character constants that makes it a bit hard to read. I'd parenthesize the comparisons: (and maybe move the last value in line.)

Vehicle new_vehicle = (arg == 'B') ? bus      :
                      (arg == 'A') ? airplane :
                      (arg == 'T') ? train    :
                      (arg == 'C') ? car      :
                      (arg == 'H') ? horse    :
                                     feet;

Now it looks great.

Answer 19

How about:

enum Vehicle { bus = 'B', airplane = 'A', train, car = 'C', horse = 'H', feet = 'F' };
...
new_vehicle = arg;

:-), by the way.

Answer 20

I don't particularly care for it.

It doesn't really buy anything, or make anything more clear, and it's a pretty non-standard usage of the operator.

It seems the primary advantage is that it's somewhat clever. I avoid clever unless there's a pretty good (external) reason to be clever.

Answer 21

I think its useful for someone who code it, but will be difficult to understand for the reviwer,

"KEEP IT SIMPLE BUDDY"