2

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?

10
  • 6
    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, 2014 at 10:40
  • 1
    @DanAllen, in all fairness you haven't seen the original code.
    – P i
    Nov 27, 2014 at 10:41
  • 1
    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.
    – user743382
    Nov 27, 2014 at 10:43
  • 2
    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.
    – user743382
    Nov 27, 2014 at 10:50
  • 1
    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! Nov 27, 2014 at 10:57

1 Answer 1

8

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.

4
  • 1
    Sometimes I wish the website allowed more than one upvote. Thanks!
    – P i
    Nov 27, 2014 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, 2014 at 13:54
  • 1
    @Pi, there is also some standard approach, by using std::rel_ops 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, 2014 at 16:27
  • 1
    @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, 2014 at 17:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.