Unify old and new interfaces in a single class #970
Conversation
| class BruteForce | ||
| : public BasicBruteForce<MemorySpace, Details::PairIndexVolume<Box>, | ||
| Details::DefaultIndexableGetter, BoundingVolume> | ||
| template <typename MemorySpace> |
There was a problem hiding this comment.
This changes the number of template arguments and actually breaks backwards compatibility if someone was using ArborX::BruteForce<MemorySpace, BoundingVolume>. But to my knowledge, no one had done it, as we have not documented it.
aprokop
force-pushed
the
the_great_unification
branch
from
f59c7ee to
6516ac3
Compare
| class BruteForce | ||
| : public BasicBruteForce<MemorySpace, Details::PairIndexVolume<Box>, | ||
| Details::DefaultIndexableGetter, BoundingVolume> | ||
| template <typename MemorySpace> |
| : public BasicBruteForce<MemorySpace, Details::PairIndexVolume<Box>, | ||
| Details::DefaultIndexableGetter, BoundingVolume> | ||
| template <typename MemorySpace> | ||
| class BruteForce<MemorySpace, Details::LegacyDefaultTemplateValue, |
There was a problem hiding this comment.
Why did you give a default template argument to Value in the primary template then?
There was a problem hiding this comment.
So that ArborX::BruteForce<MemorySpace> and ArborX::BoundingVolumeHierarchy<MemorySpace> still works. If you try to remove it, you get things like
test/tstQueryTreeCallbacks_BF.cpp:5:39: error:
too few template arguments for class template 'BruteForce'
using ArborX_BruteForce_Box = ArborX::BruteForce<MemorySpace>;
There was a problem hiding this comment.
template <class MS>
using BF = ArborX::BruteForce<MemorySpace>;
template <class MemorySpace>
using ArborX_BruteForce_Box = BF<MemorySpace>;There was a problem hiding this comment.
I don't understand it. If a user had a code like ArborX::BruteForce<MemorySpace> blah, your code does nothing to it. For example, examples/brute_force would simply break.
Just looking for feedback for this way of unification. Essentially, it introduced an (undocumented and unused) default value to the
Valuetemplate argument, and then hacks the inherited legacy class based on that.