Tidying up C++ operator overloads

Question

I have been tidying up some old C++ code.

I've reduced pages of operator overload functions using a couple of macros:

.hpp

    // for context, Long and Object wrap respective Python primitives
    class Long: public Object
    {...};    

#define OPS( A, B ) \
    bool operator == ( A, B ); \
    bool operator != ( A, B ); \
    bool operator >  ( A, B ); \
    bool operator <  ( A, B ); \
    bool operator >= ( A, B ); \
    bool operator <= ( A, B );

#define UNI( A ) \
    OPS( A, A )

#define BI_( A, B ) \
    OPS( A, B ) \
    OPS( B, A )

    UNI( const Long&  )
    BI_( const Long& , int )
    BI_( const Long& , long )

    UNI( const Float&  )
    BI_( const Float& ,  double )

#undef BI_
#undef UNI
#undef OPS
#undef OP

.cpp

#define OP( op, l, r, cmpL, cmpR ) \
    bool operator op( l, r ) { return cmpL op cmpR; }

#define OPS( ... ) \
    OP( != , ##__VA_ARGS__ ) \
    OP( == , ##__VA_ARGS__ ) \
    OP( >  , ##__VA_ARGS__ ) \
    OP( >= , ##__VA_ARGS__ ) \
    OP( <  , ##__VA_ARGS__ ) \
    OP( <= , ##__VA_ARGS__ )

#define BI_( a, b, convA, convB ) \
    OPS( a, b, convA, convB ) \
    OPS( b, a, convB, convA )

    OPS( const Long&  a, const Long&  b  ,  a.as_long()      , b.as_long()    )
    BI_( const Long&  a, int          b  ,  a.as_long()      , b              )
    BI_( const Long&  a, long         b  ,  a.as_long()      , b              )

    OPS( const Float& a, const Float& b  ,  a.as_double()    , b.as_double()  )
    BI_( const Float& a, double       b  ,  a.as_double()    , b              )

#undef BI_
#undef OPS
#undef OP

It still doesn't feel quite right.

It spreads the class over three separate locations: the class declaration itself, declarations for the operators later in the same header, and then in a separate .cxx file the actual definitions.

Is there a cleaner way to implement these operators in C++11?

If that's tidying up, then you and I have very different ideas of what tidy is. I'm not judging, just sayin'. — Persixty, Nov 27 '14 at 10:40
@DanAllen, in all fairness you haven't seen the original code. — P i, Nov 27 '14 at 10:41
This looks like it would be acceptably short, and far more readable, by simply getting rid of all the macros and writing it out fully. — , Nov 27 '14 at 10:43
I don't think either the full original version or my macro version are satisfactory. That's why I'm asking here. — P i, Nov 27 '14 at 10:45
I realise. I suppose I'm thinking you either unravel the macros or decide (as they say) you can't polish a turd. — Persixty, Nov 27 '14 at 10:47
You might avoid operators dealing with `Float` and `double` by adding implicit conversions from `double` to `Float` or vice versa (but not both, to avoid introducing ambiguities). Same for `Long` and `long`. When you then no longer have operators that take a primitive type as their LHS, the operators can become member functions, and when the operators become member functions, they may be implemented in a templated base class to avoid repetition. — , Nov 27 '14 at 10:50
If you'd have unit tests, you could do any refactoring you want. When I start tidying up, I start writing tests. And when I have to stop in between and my code looks like yours above, no problem, I can continue cleaning code *anytime*. I have unit tests! — TobiMcNamobi, Nov 27 '14 at 10:57
@TobiMcNamobi, sure I have unit tests. But that doesn't relate to the question. — P i, Nov 27 '14 at 11:20
@P i Well, having unit tests means it will be save to try out anything - whatever answer you get here. Just wanted to make sure ... ;-) — TobiMcNamobi, Nov 27 '14 at 11:50
@hvd, that solves the problem perfectly, thank you! Could you repaste that comment as an answer so that I can accept it? — P i, Nov 27 '14 at 13:38

score 8 · Accepted Answer · edited Nov 27 '14 at 15:00

Why not use a CRTP base class to provide all the operators?

template<typename C, typename T>
struct ops_base
{
  friend bool operator==(const ops_base& l, const ops_base& r)
  {
    const C& cl = static_cast<const C&>(l);
    const C& cr = static_cast<const C&>(r);
    return (caster<C, T>::cast(cl) == caster<C, T>::cast(cr));
  }

  friend bool operator==(const ops_base& l, T r)
  {
    const C& cl = static_cast<const C&>(l);
    return (caster<C, T>::cast(cl) == r);
  }

  friend bool operator==(T l, const ops_base& r)
  {
    const C& cr = static_cast<const C&>(r);
    return (l == caster<C, T>::cast(cr));
  }

  friend bool operator!=(const ops_base& l, const ops_base& r)
  { return !(l == r); }

  friend bool operator!=(const ops_base& l, T r)
  { return !(l == r); }

  friend bool operator!=(T l, const ops_base& r)
  { return !(l == r); }

  friend bool operator<(const ops_base& l, const ops_base& r)
  {
    const C& cl = static_cast<const C&>(l);
    const C& cr = static_cast<const C&>(r);
    return (caster<C, T>::cast(cl) < caster<C, T>::cast(cr));
  }

  friend bool operator<(const ops_base& l, T r)
  {
    const C& cl = static_cast<const C&>(l);
    return (caster<C, T>::cast(cl) < r);
  }

  friend bool operator<(T l, const ops_base& r)
  {
    const C& cr = static_cast<const C&>(r);
    return (l < caster<C, T>::cast(cr));
  }

  friend bool operator>(const ops_base& l, const ops_base& r)
  {
    const C& cl = static_cast<const C&>(l);
    const C& cr = static_cast<const C&>(r);
    return (caster<C, T>::cast(cl) > caster<C, T>::cast(cr));
  }

  friend bool operator>(const ops_base& l, T r)
  {
    const C& cl = static_cast<const C&>(l);
    return (caster<C, T>::cast(cl) > r);
  }

  friend bool operator>(T l, const ops_base& r)
  {
    const C& cr = static_cast<const C&>(r);
    return (l > caster<C, T>::cast(cr));
  }

  friend bool operator<=(const ops_base& l, const ops_base& r)
  { return !(l > r); }

  friend bool operator<=(const ops_base& l, T r)
  { return !(l > r); }

  friend bool operator<=(T l, const ops_base& r)
  { return !(l > r); }

  friend bool operator>=(const ops_base& l, const ops_base& r)
  { return !(l < r); }

  friend bool operator>=(const ops_base& l, T r)
  { return !(l < r); }

  friend bool operator>=(T l, const ops_base& r)
  { return !(l < r); }
};

And then use it like this:

struct Long : ops_base<Long, long>
{
  Long(long val) : value_(val) { }

  long as_long() const { return value_; }

private:
  long value_;
};

struct Float : ops_base<Float, double>
{
  Float(double val) : value_(val) { }

  double as_double() const { return value_; }

private:
  double value_;
};

Running example on ideone.

Sometimes I wish the website allowed more than one upvote. Thanks! — P i, Nov 27 '14 at 13:51
Also +1 for hvd's comment, which I followed to the letter achieving a very clean solution. (I also needed to make sure Float and Long each a maximum of 1 non-explicit type-conversion function to avoid "ambiguous resolution" errors from the compiler. — P i, Nov 27 '14 at 13:54
@Pi, there is also some standard approach, by `using std::rel_ops` http://en.cppreference.com/w/cpp/utility/rel_ops/operator_cmp , which define lots of derived operators for you in terms of `=` and `<`. You still have to define `=` and `<` though. +1 for the CRTP approach. — vsoftco, Nov 27 '14 at 16:27
@Pi, although `using namespace std::rel_ops` is kind of flawed, as it overloads the ops for ALL classes, and moreover, does not allow ADL. So the elegant solution above is the best bet. — vsoftco, Nov 27 '14 at 17:24

Tidying up C++ operator overloads

1 Answers1

Linked