tags:

views:

242

answers:

6
+5  A: 

As you discovered, a bool is really an int. The compiler is picking the correct function for your footprint. If you want to keep similar syntax, you might try

char*gdh=0;
dMessage.addAVP(AVP\_DESTINATION\_HOST, 
                (gdh=peer->getDestinationHost()) ? gdh : peer->getHost());

I would strongly recommend against redefining the operator. From a maintenance perspective, this is very likely to confuse later developers.

atk
A: 

You could instead do something like:

dMessage.addAVP(AVP_DESTINATION_HOST, (peer->getDestinationHost())? peer->getDestinationHost() : peer->getHost());

This is not as neat as || but near to it.

VNarasimhaM
Note that in this case, peer->getDestinationHost() is called twice, which might not be desirable.
Edan Maor
+4  A: 

Why are you using an "or" operator on two char pointers?

I am assuming that peer->getDestinationHost() or peer->getHost() can return a NULL, and you are trying to use the one that returns a valid string, right?

In that case you need to do this separately:


char *host = peer->getDestinationHost();
if(host == NULL)
  host = peer->getHost();
dMessage.addAVP(AVP\_DESTINATION\_HOST, host);

It makes no sense to pass a boolean to a function that expects a char *.

In C++ || returns a bool, not one of its operands. It is usually a bad idea to fight the language.

Dima
+1, because this is the most readable version of all the answers here. I find the attempt to use || (and even ?:) in a context like this to be more clever than clear.
Adrian McCarthy
+1  A: 

Well, you're right about what the problem is with your code: a || b will return a bool, which is converted to int (0 for false, != 0 for true).

As for your questions:

  1. I'm not sure whether the return value is actually defined in the standard or not, but I wouldn't use the return value of || in any context other than a bool (since it's just not going to be clear).

  2. I would use the ? operator instead. The syntax is: (Expression) ? (execute if true) : (execute if false). So in your case, I'd write: (peer->getDestinationHost() =! NULL) ? peer->getDestinationHost() : peer->getHost(). Of course, this will call getDestinationHost() twice, which might not be desirable. If it's not, you're going to have to save the return value of getDestinationHost(), in which case I'd just forget about making it short and neat, and just use a plain old "if" outside of the function call. That's the best way to keep it working, efficient, and most importantly, readable.

Edan Maor
+6  A: 

It is legal in C++ to overload the logic operators, but only if one or both of the arguments are of a class type, and anyway it's a very bad idea. Overloaded logic operators do not short circuit, so this may cause apparently valid code elsewhere in your program to crash.

return p && p->q;  // this can't possibly dereference a null pointer... can it?
Dave Hinton
I've accepted this answer since the fact that overloaded logic operators do not short circuit seems to me as the only argument against doing it, since performance-wise it can be made just as fast as a solution based on the ternary operator a?b:c.
Joao Tavora
+2  A: 

1) Should I be using this kind of shortcut, i.e. using the ||-operator's return value, at all in C++. Is this compiler dependent?

It's not compiler dependent, but it doesn't do the same as what the || operator does in languages such as JavaScript or or in common lisp. It coerces it first operand to a boolean values, and if that operand is true returns true. If the first operand is false, the second is evaluated and coerced to a boolean value, and this boolean value is returned.

So what it is doing is the same as ( peer->getDestinationHost() != 0 ) || ( peer->getHost() != 0 ). This behaviour is not compiler dependent.

2) Imagine that I really, really had to write the nice a || b syntax, could this be done cleanly in C++? By writing an operator redefinition? Without losing performance?

Since you are using pointers to chars, you can't overload the operator ( overloading requires one formal parameter of a class type, and you've got two pointers ). The equivalent statement C++ would be to store the first value in a temporary variable and then use the ?: ternary operator, or you can write it inline with the cost of evaluating the first expression twice.

Pete Kirkham