2

I am working in C++. I have two statements involving the ternary operator. 

std::stack<Node*> s;
int depth = 0;
/*modify depth integer and stack size, based on height of BST*/
depth = s.size() > depth ? s.size() : depth;

std::stack<Node*> s;
int depth = 0;
/*modify depth integer and stack size, based on height of BST*/
s.size() > depth ? depth = s.size() : depth = depth;

My question is this: Is there a standard way to assign a variable with the ternary operator? Is one of these forms more efficient, concise, or better than the other?

The second example seems to be more redundant than the first.

Edit: added comment that both s and depth are modified before the ternary operator.

lfitz
  • 35
  • 5
  • 4
    I rarely see the second one in the wild. Mostly the first. The compiler should convert both to code resembling the second. – Millie Smith Jul 13 '16 at 05:43
  • 4
    `int depth = s.size();` would be enough here. – songyuanyao Jul 13 '16 at 05:44
  • 7
    `depth = std::max(depth, s.size())` – Brian Bi Jul 13 '16 at 05:46
  • 1
    @songyuanyao he's taking the max of the two. I'm assuming 0 and the unpopulated stack are just placeholders. – Millie Smith Jul 13 '16 at 05:46
  • 4
    Not related, but depth shoutd be a `std::size_t` to have the same type as s.size(). – Boiethios Jul 13 '16 at 05:46
  • 7
    The second form violates several common style-guide rules, such as using an expression as a statement, and inner expressions with side-effects. And obscurity. Not that style guides are infallible. – user207421 Jul 13 '16 at 05:47
  • @MillieSmith You might be right. The sample code is not clear enough. – songyuanyao Jul 13 '16 at 05:49
  • 1
    Another issue with the second form is that it's not immediately obvious that it does the correct thing: there is some complicated situation with respect to the precedence of the conditional and assignment operators. In this case it does the right thing, but you have to think about it carefully. It's not very readable code. – Brian Bi Jul 13 '16 at 05:50
  • 3
    Both are terrible! Just `if(s.size() > depth) depth = s.size();` to make it readable. –  Jul 13 '16 at 05:57
  • Maybe using `0` in the example isn't a great idea. Otherwise it could be simplified to `size_t depth = s.size();` – juanchopanza Jul 13 '16 at 06:09
  • if we interpret the code literally, then we can throw out both tertiary lines. s.size() is always 0 when interpreting the code literally. – Millie Smith Jul 13 '16 at 06:24

2 Answers2

2

The ternary expression is primarily used for the value that it produces. It's possible to abuse it and only use its side effects, but that kind of construct is better left to an ordinary if statement. So the first example is an idiomatic use of the ternary expression (although, as the comments point out, probably inappropriate in this particular case), while the second example is not.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
0

The first example is more idiomatic than the second. This is what I was looking for.

Note: However, after reading, I agree this is poor use of the operator. A simple if statement works in place of the ternary operator.

lfitz
  • 35
  • 5