branch vs ternary... cast

[CodeMonkey]

This code gives me no problem, where I conditionally set virtual pointers to instances of derived types:

    thread_method * tm;
    mode_method * mm;

    if(nthreads>1)
		tm = new multi_thread(nthreads,reg);
    else
		tm = new single_thread;
    if(settings::mode_to_find == 0)
		mm = new low_mode;
    else
		mm = new high_mode(*tm,modes);

However, in the following code, I get an error urging me to apply casts to "fundamentally different pointer types." Why doesn't the class hierarchy see through the ternary operator's desire to be of one type?

tm = (nthreads>1) ? new multi_thread(nthreads,reg) : new single_thread;
mm = settings::mode_to_find ? new high_mode(*tm,modes) : new low_mode;

[CornedBee]

Simply put, the rules in the standard about the result type of the ternary operator don't allow it. The critical phrase is actually in 5.9p2: "Otherwise the composite pointer type is a pointer type similar (4.4) to the type of one of the operands".
This means that the underlying type of the result pointer type must be the same as that of one of the argument pointer types.

[matsp]

If I understand what you are trying to do, I'd say it doesn't matter: Using a ternary operator here is probably pointless, as it's EXTREMELY unlikely that the compiler can produce branch-less code to call new in two different ways from the code you supplied. So if you are trying to avoid branches, then you are not succeeding [to verify this, you should, of course, check the assembler produced by the compiler, but I'd bet a couple of pence on "the code is identical for if-else vs ?: code variants".

On the other hand, it is much easier to read the code with proper if-statements.

Ternary operator MAY make the compiler more likely to make a branchless version of the statement when the condition and resulting values are really simple - assuming the processor has instructions that allow it to be done in the first place. This would be for example a CMOVcc instruction in X86. ARM instructions have the ability to "conditionalize" almost any instruction, but you do not want to have TOO long a sequence of instructins that aren't doing anything, because it still takes clock-cycles to perform each instructin, with nothing actually getting done. It gets even more complicated if the resulting code causes condition flags to change.

--
Mats

[CodeMonkey]

I see. Thanks.
On the contrary, I wasn't trying to avoid branches, but increase readability. I personally find the two lines of ternary stuff cleaner than the if/else. Apparently you disagree with me.

I'm just looking for ways to shorten my code without compromising clarity.

[brewbuck]

I see. Thanks.
On the contrary, I wasn't trying to avoid branches, but increase readability. I personally find the two lines of ternary stuff cleaner than the if/else. Apparently you disagree with me.

I'm just looking for ways to shorten my code without compromising clarity.

The ternary operator can make code clearer in certain circumstances. Where the two branches of the operator evaluate to different expression types, IMHO, is not one of those circumstances.

To avoid code clutter for very simple conditionals and antecedents, I sometimes combine the conditional and antecedent onto one line:

if(condition1) x = something;
else if(condition2) x = something_else;
else x = yet_another_thing;

[CodeMonkey]

Yes, that's worth a look. I used to do that but was urged to use two lines for better readability. It all depends on the code and the coder, I guess.

[matsp]

I see. Thanks.
On the contrary, I wasn't trying to avoid branches, but increase readability. I personally find the two lines of ternary stuff cleaner than the if/else. Apparently you disagree with me.

I'm just looking for ways to shorten my code without compromising clarity.

There are two reasons I think that ternary operator is less good than if-else:
1. You do not IMMEDIATELY see that there is a condition involved - only once you get to the questionmark does it become a condition. A line starting with "if" is clearly something to do with conditions!
2. When debugging, you can not see what path the code takes [at least, most debuggers treat an expression with ternary expression as a single unit], without resorting to assembler debugging (or setting breakpoints in places that may get called through the ternary expression, but that again complicates matters by a fair amount).

In my mind, clearly seeing where the code may divert from "straight on" is a good thing. Being able to understand how the code works by stepping through it and/or setting breakpoints on a line inside an if/else expression is often critical to understanding why something works the way it does - and you may not have had the experience of working on LARGE projects where you are told to fix a bug in some code that isn't written by yourself (and perhaps not even written by your company), or has been modified from when you wrote it by someone who didn't quite use the same coding style as yourself, or any of the other possible scenarios where code just isn't quite clear when it follows one or another path.

--
Mats

[phantomotap]

I think applying a single layer of the conditional operator can greatly improve source readability when the usage guarantees "good" construction. Applying a few layers of the conditional operator can improve or simplify exception characteristics. (You don't have to cache constructor parameter values for later construction. You don't have to delay construction. You don't have to reassign an already constructed object. You don't have to check for a "partially constructed" object. You don't have to compromise the purity of an object to allow for a "partially constructed" state. You don't have to offside construction.) Without having to rely on some source variation to avoid the conditional operator you may freely code under the assumption that any object is "good" at the terminating ';' of the declaration without separating the value logic from the location it's used.

I don't disagree with brewbuck and matsp. The conditional operator can cloud intent. It can also clarify intent. I certainly don't suggest granting nested occasions a pass just because a single layer may pass.

You should consider every case carefully.

I think I'm responsible for putting the particulars here in his brain. I've provided an example of how to do it without making the source brittle in the face of simple changes or additions.

Soma

#include <iostream>
#include <memory>
#include <string>

struct base
{
   virtual void go() const = 0;
};

struct derived: public base
{
   virtual void go() const
   {
      std::cout << "derived::go\n";
   }
};

struct derived_magic: public base
{
   virtual void go() const
   {
      std::cout << "derived_magic::go\n";
   }
};

int main
(
   int argc,
   char ** argv
)
{
   typedef std::auto_ptr<base> base_aptr;
   std::string derived_choice((2 == argc) ? (argv[1]) : ("magic"));
   base_aptr p(("magic" == derived_choice) ? (base_aptr(new derived_magic)) : (base_aptr(new derived)));
   p->go();
   return(0);
}

[matsp]

I do not think that is a particularly good example of where ternary operators should be used.

Sure, there are situations when they make life simpler:
1. Return statement for a simple case of testing (e.g. max).
2. Function calls with many arguments where one or two differs based on a simple condition.

For nearly everything else, an if/else statement is better. If nothing else because it is more expandable.

--
Mats

[phantomotap]

I do not think that is a particularly good example of where ternary operators should be used.

I think improving readability, simplifying flow, reducing one-shot variables, guaranteeing "good" construction, and possibly simplifying exception characteristics are the best reasons to do virtually anything.

You are free to opine that the use of the conditional operator can't do any of these things if you like. You'll just be wrong. (Any particular case may do none of these things or all of them.)

Function calls with many arguments where one or two differs based on a simple condition.

It's transformation time kiddies!

0) Let's assume there is a large number of default parameters.

base_aptr p(("magic" == derived_choice) ? (base_aptr(new derived_magic)) : (base_aptr(new derived)));

1) Let's get rid of those extra parentheses I like to use.

base_aptr p("magic" == derived_choice ? base_aptr(new derived_magic) : base_aptr(new derived));

2) A constructor is mechanically just a function call so let's make them look like a function call.

go("magic" == derived_choice ? go_derived_magic() : go_derived());

3) The string "magic" is just a magic value but let's use a magic integer value instead.

go(0 != choice ? go_derived_magic() : go_derived());

4) Now let's remove the comparison.

go(choice ? go_derived_magic() : go_derived());

5) Now let's pretend we stored the results.

go(choice ? magic_derived : derived);

6a) Let's just go ahead and offside the mechanic entirely.

go(get_something());

Would you look at that? You've hidden this possibly case specific value logic behind a probably pointless function. (I honestly find nothing wrong with such functions, but that's not the point.)

6b) Let's just offside the mechanic without changing the condition.

go(get_choice_of_derived(choice));

Would you look at that? You are copying an object. (Not every object can be copied.)

7a) Let's use a normal condition instead.

go_param_type p;
if(choice)
{
   p = magic_derived;
}
else
{
   p = derived;
}
go(p);

Would you look at that? You've reassigned an already constructed object. (Not every object can be reassigned.) What if the object is expensive to construct? What if reassigning the object is expensive?

7b) Let's use a pointer and delay construction.

go_param_type * p;
if(choice)
{
   p = create_go_param_type_from_magic_derived(magic_derived);
}
else
{
   p = create_go_param_type_from_derived(derived);
}
go(p);

Would you look at that? You've introduced a bug.

7c) Let's wrap everything to provide exception safety.

RAII_go_param_type p;
if(choice)
{
   p = create_go_param_type_from_magic_derived(magic_derived);
}
else
{
   p = create_go_param_type_from_derived(derived);
}
go(p.get());

Would you look a that? Sexy! Boy am I glad we didn't use the conditional operator.

For nearly everything else, an if/else statement is better.

My opinion happens to be different.

Soma

[matsp]

When I said "long list of arguments", I meant something along the lines of this - say you want to create a new file or open an existing based on "newFile" flag:

HANDLE h =  CreateFile("myfile.txt", GENERIC_READ | GENERIC_WRITE, 
                                      0, NULL, (newFile)?CREATE_ALWAYS:OPEN_EXISTING, 
                                      FILE_ATTRIBUTE_NORMAL, NULL);

I accept that not everyone has the same point of view.

--
Mats

[phantomotap]

CFILEHANDLE h("myfile.txt", GENERIC_READ | GENERIC_WRITE, 
                                      0, NULL,
("magic" == derived_choice)?(base_aptr(new derived_magic)):(base_aptr(new derived)),
                                      FILE_ATTRIBUTE_NORMAL, NULL);

There. You have a long list of arguments. The conditional operator is still being used to determine which value (which of the available arguments targeted at a particular parameter) is being passed to a function.

Soma

[matsp]

Sure, that is a valid use - it avoids duplicating three or more lines of code that only differ by a tiny bit. The alternative would be to have a variable to hold a temporary value from an if-statement before the constructor call.

I would also say that ternary operator should be used sparingly - it should be the exception, not the rule, if you see what I mean.

--
Mats

[iMalc]

There are other options, but they aren't any prettier:

(nthreads>1 && (tm = new multi_thread(nthreads, reg)) || (tm = new single_thread));
(settings::mode_to_find) && (mm = new high_mode(*tm,modes)) || (mm = new low_mode);

[matsp]

There are other options, but they aren't any prettier:

(nthreads>1 && (tm = new multi_thread(nthreads,reg)) || (tm = new single_thread));
(settings::mode_to_find) && (mm = new high_mode(*tm,modes)) || (mm = new low_mode);

Given a choice, I prefer the ternary operator - but yes, that is an option in some cases.

--
Mats