The conditional operator is not allowing the program to terminate

Question

I just learnt about conditional operators and was doing an introductory exercise stating:

Write a program to use a conditional operator to find the elements in a vector<int> that have odd value and double the value of each such element.

Here is the code that I wrote:

int main()
{
    vector<int> nums = { 1,2,3,4,5,6,7,8,9 };
    int i;
    auto beg = nums.begin();

    while (*beg > 0) // This will always evaluate to true. 
    { 
        ((*beg) % 2 == 0 && (beg < nums.end()) ? i = 0 : *beg = 2 * (*(beg++)));
        /*If the number is even the program will just assign 0 to i*/
    }
}

The program terminates AND gives you the correct output if you change the last line to:

((*beg)%2 == 0 && (beg < nums.end()) ? i = 0 : *beg = 2*(*(beg))); 
++beg;

Why is this happening?

Because in in the first version the increment is called only if `((*beg)%2 == 0 && (beg < nums.end())` is `false`, so your program get stuck. In the second version the increment is always called — Butiri Dan, Aug 17 '19 at 09:09
@ButiriDan Why would it get stuck? It can always assign i to 0 if the condition turned out to be true! — K.Oler, Aug 17 '19 at 09:16

score 2 · Accepted Answer · answered Aug 17 '19 at 09:23

It stuck because, if the condition ((*beg)%2 == 0 && (beg < nums.end()) is true, the iterator will not increment for checking further. You have only setting i=0. You should increment the iterator as well.

You can use comma operator for this:

while (beg != nums.end()  && *beg > 0)
{
    (*beg) % 2 == 0  ? (beg++, i): (*beg = 2 * (*beg) , beg++, ++i );
}

Also note that the count i should be initialized before-hand, not in the while loop. The complete working code as per the requirement would be:

#include <iostream>
#include <vector>

int main()
{
    std::vector<int> nums = { 1,2,3,4,5,6,7,8,9 };
    int i{0};
    auto beg = nums.begin();

    while (beg != nums.end()  && *beg > 0)
    {
        (*beg) % 2 == 0  ? (beg++, i): (*beg = 2 * (*beg) , beg++, ++i );
    }
    for (const int ele : nums)
            std::cout << ele << " ";
    std::cout << "\ncount: " << i << "\n";
}

Output:

2 2 6 4 10 6 14 8 18 
count: 5

That being said, IMO using comma operator along with conditional operator like the above(the task) is not a good coding manner, which will only make confusions for the future readers of your codebase.

Also read: Why is "using namespace std;" considered bad practice?

score 0 · Answer 2 · answered Aug 17 '19 at 12:45

If you want to double some values and not others, just do it:

#include <iostream>
#include <vector>

int main() {
    std::vector<int> nums = { 1, 2, 3, 4, 5, 6, 7, 8, 9 };
    for (int& num : nums)
        num = num % 2 ? 2 * num : num;
    for (int num : nums)
        std::cout << num << ' ';
    std::cout << '\n';
    return 0;
}

A conditional expression is an expression; you use it to compute a value. The code in the question does not do that; it uses the conditional expression as a way of selecting side effects, which is better done with an ordinary if statement.

The conditional operator is not allowing the program to terminate

2 Answers2