current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite
2

I'm attempting to tidy up a C++ framework (PyCXX).

One particular file contains ~400 lines of operator overload functions, which I've managed to reduce to the following:

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

#define OPS( l, r, cmpL, cmpR ) \
    OP( !=, l, r, cmpL, cmpR ) \
    OP( ==, l, r, cmpL, cmpR ) \
    OP( > , l, r, cmpL, cmpR ) \
    OP( >=, l, r, cmpL, cmpR ) \
    OP( < , l, r, cmpL, cmpR ) \
    OP( <=, l, r, cmpL, cmpR )

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

#ifdef HAVE_LONG_LONG
    OPS( const Long &a, PY_LONG_LONG b,   a.as_long_long() , b                )
    OPS( PY_LONG_LONG a, const Long &b,   a                , b.as_long_long() )
#endif

    //------------------------------------------------------------
    // compare operators
    OPS( const Float &a, const Float &b,   a.as_double() , b.as_double() )
    OPS( const Float &a, double       b,   a.as_double() , b             )
    OPS( double a,       const Float &b,   a             , b.as_double() )

}    // end of namespace Py

However, I wonder if it may be possible to tidy it up further.

It looks as though I can save having to pass all the parameters each time by using:

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

Although I have a hunch this might be making the code less transparent.

Another possible idea would involve storing pairs:

#define FD(op, x) (       Float &x , op, x.as_double()    )
#define L1(op, x) ( const Long &x  , op, x.as_long()      )
#define L2(op, x) ( const Long &x  , op, x.as_long_long() )
:

but I am risking obfuscation through trying too hard. I think it is commonly accepted that other things being equal, readability trumps concise.

What other viable design patterns could I consider?

share|improve this question
    
@glampert, looks like you can't distinguish between member and non-member operators; that's sad. –  Griwes Oct 6 '14 at 9:15
    
Adding 'const' gives: "Non-member function cannot have 'const' qualifier" –  P i Oct 6 '14 at 9:37
1  
Could you use templates? –  David K Oct 6 '14 at 11:38
    
You could use some recursive macros, but I would personally create a script to generate the file from another file (specifying e.g. type; op1,op2,op3; arg1,arg2,arg3 - generator will emit all possible combinations). –  firda Oct 6 '14 at 12:28
2  
@Griwes, people make mistakes, no need to be rude about it... –  glampert Oct 6 '14 at 13:45

2 Answers 2

up vote 5 down vote accepted

I was looking a bit at your PyCXX and want to offer you Barton–Nackman trick using Curiously recurring template pattern together with variadic template recursion (through partial specialization). This way you can solve all your types and conversion operators with one template:

template<typename T> struct Arithmetic;
using Long = Arithmetic<long long>;
using Double = Arithmetic<long double>;

template<class Base, class... Other> struct compare_operators {};

template<class Base, class First, class... Other>
  struct compare_operators<Base, First, Other...>
  : compare_operators<Base, Other...>
{
    friend bool operator == (const Base& a, First b) {
        return a.template as<First>() == b; }
    friend bool operator != (const Base& a, First b) {
        return a.template as<First>() != b; }
    friend bool operator <  (const Base& a, First b) {
        return a.template as<First>() <  b; }
    friend bool operator <= (const Base& a, First b) {
        return a.template as<First>() <= b; }
    friend bool operator >  (const Base& a, First b) {
        return a.template as<First>() >  b; }
    friend bool operator >= (const Base& a, First b) {
        return a.template as<First>() >= b; }

    friend bool operator == (const Base& a, const Arithmetic<First>& b) {
        return a.template as<First>() == b.value; }
    friend bool operator != (const Base& a, const Arithmetic<First>& b) {
        return a.template as<First>() != b.value; }
    friend bool operator <  (const Base& a, const Arithmetic<First>& b) {
        return a.template as<First>() <  b.value; }
    friend bool operator <= (const Base& a, const Arithmetic<First>& b) {
        return a.template as<First>() <= b.value; }
    friend bool operator >  (const Base& a, const Arithmetic<First>& b) {
        return a.template as<First>() >  b.value; }
    friend bool operator >= (const Base& a, const Arithmetic<First>& b) {
        return a.template as<First>() >= b.value; }
};

template<typename T> struct Arithmetic
  : compare_operators<Arithmetic<T>
  , long long, int, short, double, float, long double>
{
    T value;
    Arithmetic(T value): value(value) {}

    template<typename X, typename = enable_if_t<is_arithmetic<decay_t<X>>::value>>
      X as() const { return (X)value; }
};

int main() {
    Long val(3);
    if(val > 2) cout << "val > 2\n";
    if(val <= 3.2) cout << "val <= 3.2\n";
    Double dbl(3.14);
    if(val < dbl) cout << "val < dbl\n";
}

Beware that the comparision operators need some fine-tuning especially when you compare integral vs. floating-point (which to choose as common type) and that dacay_t in conversion operator may be problematic (not sure). (Finally imagine using namespace std, but I use my own header while explicitly using selected std features/classes in the namespace and then using the namespace).

BTW: This would be the same (but the as conversion is altered) in my own syntax, which you may find a bit familiar:

#include "basics.hpp"
using*firda

forward template<typename T> struct Arithmetic
using Long = Arithmetic<long long>
using Double = Arithmetic<long double>

template<class Base, class... Other>
  struct compare_operators

template<class Base, class First, class... Other>
  struct compare_operators<Base, First, Other...>
  : compare_operators<Base, Other...>

    friend bool operator == (const Base& a, First b)
        return a.template as<First>() == b
    friend bool operator != (const Base& a, First b)
        return a.template as<First>() != b
    friend bool operator <  (const Base& a, First b)
        return a.template as<First>() <  b
    friend bool operator <= (const Base& a, First b)
        return a.template as<First>() <= b
    friend bool operator >  (const Base& a, First b)
        return a.template as<First>() >  b
    friend bool operator >= (const Base& a, First b)
        return a.template as<First>() >= b

    friend bool operator == (const Base& a, const Arithmetic<First>& b)
        return a.template as<First>() == b.value
    friend bool operator != (const Base& a, const Arithmetic<First>& b)
        return a.template as<First>() != b.value
    friend bool operator <  (const Base& a, const Arithmetic<First>& b)
        return a.template as<First>() <  b.value
    friend bool operator <= (const Base& a, const Arithmetic<First>& b)
        return a.template as<First>() <= b.value
    friend bool operator >  (const Base& a, const Arithmetic<First>& b)
        return a.template as<First>() >  b.value
    friend bool operator >= (const Base& a, const Arithmetic<First>& b)
        return a.template as<First>() >= b.value

template<typename T> struct Arithmetic
  : compare_operators<Arithmetic<T>
  , long long, int, short, double, float, long double>

    T value
    Arithmetic(T value): value(value) {}

    template<typename X>
      enable_if_t<is_arithmetic<X>::value,
      X> as() const
        return value

int main()
    Long val(3)
    if val > 2; cout << "val > 2\n"
    if val <= 3.2; cout << "val <= 3.2\n"
    Double dbl(3.14)
    if val < dbl; cout << "val < dbl\n"
share|improve this answer
    
Barton-Nackman is the way to solve this although I might not agree completely with your implementation. –  Emily L. Oct 6 '14 at 14:07
    
@EmilyL.: Feel free to improve it in your own answer :) This is teamwork, I don't care for points but to help people ;) –  firda Oct 6 '14 at 14:10
    
That's more time than I have to contribute right now :) But +1 anyways. –  Emily L. Oct 6 '14 at 14:58
up vote 0 down vote

This is the best I have managed to come up with:

#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              )

#ifdef HAVE_LONG_LONG
    BI_( const Long  &a, PY_LONG_LONG b  ,  a.as_long_long() , b              )
#endif

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

#undef OP, OPS, BI_

I have detailed the thought process here: http://mathpad.wikidot.com/pycxx-operators

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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