Is it dangerous to overload bool operator in this case

Question

I have seen comments or answers on SoF stating that overloading the cast operator to bool is dangerous and I should prefer the void* operator instead. Still I would like to ask if it is dangerous practice to use this operator in my use case and if yes why.

I implement a simply geometry library and one of its most basic classes is a point that I define in the following way:

struct point {
  double x, y;
  bool real;
  point(double x_, double y_): x(x_), y(y_), real(true) {}
  point(): x(0), y(0), real(true) {}

  operator bool() {
    return real;
  }
};

I will try to explain why I need the cast operator to bool. I have another class called line and I have a function declared as follows:

point intersect(const line& a, const line& b);

Now having overloaded the cast operator to bool of a point I can write both: point p = intersect(a,b); and if (intersect(a,b)) thus either getting the intersection point of two lines or checking if two lines intersect. I use this in other places as well but I believe this example is enough to show the usage of the cast operator. Is my usage of the cast operator dangerous and if it is could you please give an example when the effect will not be as expected?

EDIT: one small addition thanks to juanchopanza : in this project I am limited to not using c++11, so I can not make the operator explicit.

Answer 1

That is not good enough. Since you're stuck with C++03, safe-bool idiom is what you should be using if you need such a thing at all (otherwise, explicit conversion function should be preferred).

An alternative solution is to return boost:optional instead:

boost::optional<point> intersect(const line& a, const line& b);

I would adopt this approach (even in C++11), as it looks semantically better — parallel lines don't have intersection point, so the return value should indeed be optional (or maybe value).

Answer 2

Yes, this is dangerous. Just for example, let's consider your point exactly as-is, with no definition of operator+. Despite that lack, if we try to add two point objects, the compiler won't object at all:

point a, b;

std::cout << a + b;

This works by converting each point to bool, then adding the bools. In an integer context, false converts to 0 and true converts to 1, so we can expect the code above to print out 2. Of course, I've just used addition as an example. You could just as well do subtraction, multiplication, division, bitwise operations, logical operations, etc. All of them will compile and execute, but obviously produce worthless results. Likewise, if we pass a point to some function that only accepts a numeric type, the compiler won't stop s or complain -- it'll just convert to bool, and the function will get either 0 or 1 depending on whether real was true or not.

The safe-bool idiom is safe (well, less dangerous anyway) because you can test a void * in a Boolean context, but a void * won't implicitly convert to much of anything else like bool will. You (mostly¹) can't accidentally do arithmetic, bitwise operations, etc., on them.

---

_{1. There are a few holes anyway, mostly involving calling something else that does some sort of explicit conversion on the void *. Being able to mark conversion operators explicit is better, but honestly they give a lot more improvement in readability than safety.}

Answer 3

Strange that those people didn't tell you why overloading operator bool() is dangerous.

The reason is that C++ has implicit conversion from bool to all other numeric types. So if you overload operator bool() then you lose a lot of type checks that users of the class would normally expect. They can supply a point anywhere that an int or float is expected.

Overloading operator void*() is less dangerous because there's less that void* converts to, but still has the same fundamental issue that the void* type is used for other things.

The idea of the safe bool idiom is to return a pointer type that doesn't convert to anything used in any API.

Note that there would be no need to worry about this if you were willing to write if (intersect(a,b).real), or perhaps if (intersect(a,b).exists()). C++03 does have explicit conversions. Any function with one parameter (or non-static member function with no parameters) is a conversion of sorts. C++03 just doesn't have explicit conversion operators.

Answer 4

You might run into all kind of trouble due to implicit conversions from bool to a numeric value. Also, your line class got an (in my point of vew) dangerious member 'bool real', to carry results of function calls. An intersection function returning an intersection value or NaN if the lines do not intersect could resolve your problem.

Example

Having a (infinite) line class holding an origin p and a vector v:

struct Line {
    Point p;
    Vector v;

    Point operator () (double r) const {
        return p + r * v;
    }
};

And a function calculating an intersection value

/// A factor suitable to be passed to line a as argument to calculate the
/// inersection point.
/// - A value in the range [0, 1] indicates a point between
///   a.p and a.p + a.v.
/// - The result is NaN if the lines do not intersect. 
double intersection(const Line& a, const Line& b) {
    double d = a.v.x * b.v.y - a.v.y * b.v.x;
    if( ! d) return std::numeric_limits<double>::quiet_NaN();
    else {
        double n = (b.p.x - a.p.x) * b.v.y
                 - (b.p.y - a.p.y) * b.v.x;
        return n/d;
    }
}

Then you can do:

double d = intersection(a, b);
if(d == d) { // std::isnan is c++11
    Point p = a(d);
}

Or

if(0 <= d && d <= 1) {
    Point p = a(d);
}

Answer 5

For example, early in the C++ 2003 Standard class std::basic_ios had the following conversion operator

operator void*() const

Now as the new C++ Standard has keyword explicit this operator was substituted for

explicit operator bool() const;

So if you will add keyword explicit for your operator I think it will not be dangerous.:)

The other way is to define operator bool operator !() instead of the conversion operator.

Answer 6

Although there are already a few very good answers I would like to assemble a bit more complete answer with explanation of why the proposed solution does work.

1.Why is my solution wrong? The compiler performs implicit conversions from bool to numeric types resulting in unexpected operators being defined. For instance:

point p;
if (p < 1)

Will compile while I did not expect it to. Changing the operator for conversion to operator void*() will improve the things but only slightly as there some operators(although less than for bool) that are defined for void * and their existence will result again in unexpected behavior.

2.So what is the correct solution. The correct solution can be found here. But I will include the code for my class so that I can explain why it works and why do we solve the problem in this way:

class point {
  typedef void (point::*bool_type)() const;
  void not_supported() const; // NOTE: only declaration NO body!

public:
  double x, y;
  bool real;
  point(double x_, double y_): x(x_), y(y_), real(true) {}
  point(): x(0), y(0), real(true) {}

  operator bool_type() {
    return (real == true)? &point::not_supported : 0;
  }

  template<typename T>
  bool operator==(const T& rhs) const {
    not_supported();
    return false;
  }

  template<typename T>
  bool operator!=(const T& rhs) const {
    not_supported();
    return false;
  }
};

Now first I will explain why do we have to use a type that is a function pointer and not a plain pointer. The answer is stated by Steve Jessop in a comment below Nawaz's answer. function pointers are not < comparable, thus any attempt to write p < 5 or any other comparison between a point and some other type will fail to compile.

Why do we need to have an method with no body (not_supported)? Because in this way each time we try to instantiate the templated methods for comparison(that is == and !=), we will have a function call of a method with no body that will cause a compile error.