Reducing code duplication while defining a commutative operation

Question

I have a a set of overloads of a commutative binary function named overlap, which accepts two distinct types:

class A a; class B b;
bool overlap(A, B);
bool overlap(B, A);

My function overlap returns true if and only if one shape overlaps the other - this is one common example used when discussing multimethods.

Because overlap(a, b) is equivalent to overlap(b, a), I only need to implement one "side" of the relation. One repetitive solution is to write something like this:

bool overlap(A a, B b) { /* check for overlap */ }
bool overlap(B b, A a) { return overlap(a, b);   }

But I would prefer not to write an extra N! / 2 trivial versions of the same function by allowing them to be generated instead, using a template.

template <typename T, typename U> 
bool overlap(T&& t, U&& u) 
{ return overlap(std::forward<U>(u), std::forward<T>(t)); }

Unfortuately, this is prone to recurse infinitely, which is not acceptable: see http://coliru.stacked-crooked.com/a/20851835593bd557

How can I prevent such infinite recursion? Am I approaching the problem correctly?

Related: https://stackoverflow.com/questions/30561407/c-how-to-generate-all-the-permutations-of-function-overloads -- puts the arguments in a tuple and then uses std::get() to pull them out in the "right" order. — Eric Towers, Jul 06 '17 at 10:25
Separate from the narrow scope, I expect in many cases the need to do this is a code smell. For example, should A and B not implement a shape interface/inherit from a parent class? — Adam Martin, Jul 06 '17 at 21:29
Wouldn't it make sense to have an interface that is implemented by both of your classes, so you don't really need to care about class A and class B? Or isn't this that easy in c++? — Herr Derb, Jul 07 '17 at 07:26

Quentin · Accepted Answer · 2017-07-06T09:24:20.383

64

Here's a simple fix:

template <typename T, typename U> 
void overlap(T t, U u)
{
    void overlap(U, T);
    overlap(u, t);
}

The template itself declares the target function, which will be preferred over recursion because it is an exact match (be sure to take care of constness and reference-ness in your real case). If the function has not been implemented, you get a linker error:

/tmp/cc7zinK8.o: In function `void overlap<C, D>(C, D)':
main.cpp:(.text._Z7overlapI1C1DEvT_T0_[_Z7overlapI1C1DEvT_T0_]+0x20):
    undefined reference to `overlap(D, C)'
collect2: error: ld returned 1 exit status

... which points directly at the missing function :)

edited Jul 06 '17 at 09:24

answered Jul 06 '17 at 08:24

Quentin

62,093
7
131
191

So, in this case do you need to declare a second function that check for the overlap? – Hilder Vitor Lima Pereira Jul 06 '17 at 10:09
@Vitor what do you mean? – Quentin Jul 06 '17 at 10:13
We started with this two functions `bool overlap(A a, B b) { /* check for overlap */ }` and `bool overlap(B b, A a) { return overlap(a, b); }`. Now you propose a single function, but I don't see any code to check for overlap. I didn't understand this trick. :s – Hilder Vitor Lima Pereira Jul 06 '17 at 10:16
@Vitor no, this only replaces the second one which swaps the arguments, of course you still need to implement at least one function of each pair to perform actual work :) – Quentin Jul 06 '17 at 10:34
1

Note: it seems to me this only works for `overlap` functions in the same namespace. – Matthieu M. Jul 06 '17 at 14:07
@MatthieuM. yes. Locally-declared functions are strictly part of the enclosing namespace. – Quentin Jul 06 '17 at 14:15
Well, that's not really nice. It only works if you never inherit from any of your shape-classes, or write the neccessary forwarders. – Deduplicator Jul 06 '17 at 15:20
@Deduplicator example of first and second parts please? – Krupip Jul 06 '17 at 15:31
@Quentin In his first take of closing the gap with a template, yes. But elsewhere, no. – Deduplicator Jul 06 '17 at 15:44
@Deduplicator fair enough. I tried inejcting some SFINAE in the return type, but apparently the function template is able to refer to itself at that point already :/ – Quentin Jul 06 '17 at 18:27
2

Not detecting the error until link time could be a big downside for a large project with many source files. Changing a `.h` can lead to a lot of files needing recompilation, so link-time might be a couple minutes of compilation away. Hopefully this is easy to get right and isn't something that needs frequent re-adjustment in most code-bases. – Peter Cordes Jul 06 '17 at 18:48
This only works if the reference function is an exact match; you get a link error if, for example, the user implemented `bool overlap(const A&, const B&);`. – Jul 07 '17 at 03:17

score 12 · Answer 2 · answered Jul 06 '17 at 15:40

As a wise man once said, there is no problem you cannot solve with an additional layer of indirection, except too many layers of indirection.

So, use SFINAE and some indirection to get it done:

template<class A, class B>
auto overlap(A&& a, B&& b)
-> decltype(overlap_impl('\0', std::forward<A>(a), std::forward<B>(b)))
{ return overlap_impl('\0', std::forward<A>(a), std::forward<B>(b)); }

template<class A, class B>
auto overlap_impl(int, A&& a, B&& b)
-> decltype(do_overlap(std::forward<A>(a), std::forward<B>(b)))
{ return do_overlap(std::forward<A>(a), std::forward<B>(b)); }

template<class A, class B>
auto overlap_impl(long, B&& b, A&& a)
-> decltype(do_overlap(std::forward<A>(a), std::forward<B>(b)))
{ return do_overlap(std::forward<A>(a), std::forward<B>(b)); }

// You can provide more choices if you want, for example to use member-functions.

// Implement `do_overlap(A, B)`, maybe with references, in at least one direction.

It's nicely symmetric. I like it. – Quentin Jul 06 '17 at 18:27 — Quentin, Jul 06 '17 at 18:27
@cat: If I could use the concepts TS, maybe. – Deduplicator Jul 07 '17 at 11:53 — Deduplicator, Jul 07 '17 at 11:53

K. Kirsz · Answer 3 · 2017-07-06T08:30:01.777

1

You can rename the actual method to something like overlap_impl and call this one inside the template. I will break the recursion:

bool overlap_impl(A a, B b) { /* check for overlap */ }

template <typename T, typename U> 
bool overlap(T&& t, U&& u) 
{ return overlap_impl(std::forward<U>(u), std::forward<T>(t)); }

template<> bool overlap(A&& t, B&& u)
{ return overlap_impl(std::forward<A>(t), std::forward<B>(u)); }

edited Jul 06 '17 at 08:30

answered Jul 06 '17 at 08:24

K. Kirsz

1,384
10
11

2

You still need to forward the "right-way-around" calls to `overlap` towards `overlap_impl`. – Quentin Jul 06 '17 at 08:25
Right, well this would require the template specialisation for the right-way-around case i guess. – K. Kirsz Jul 06 '17 at 08:27
You could add some SFINAE to create just two template functions for all types which automatically figure out which order to pass the arguments. – aschepler Jul 06 '17 at 12:14

Reducing code duplication while defining a commutative operation

3 Answers3

Linked