Re: [boost] [GTL] - geometric template library - determininginterest

From: Fernando Cacciola (fernando_cacciola_at_[hidden])
Date: 2007-10-11 10:41:13

Hi Lucanus,

> Just typing off the top of my head.  I am not yet allowed to copy and
> paste the code I wrote which Intel owns.
>
> typedef int Unit;
>
> struct MyPoint {int x, int y};
>

> template <>
> class PointConceptMap<MyPoint> {
> public:
> static inline Unit PointGet(const MyPoint& t, Orientation2D
> orient) {
> return orient == HORIZONTAL ? t.x: t.y; }
>
First note.

These are static member functions, so they are equivalent to non member 
functions. Well, almost.

Template function calls performs template argument deduction, while template 
class instantiation does not.

T use this concept map, a user is required to specify not ony the class name 
but also the type:

PointConceptMap<MyPoint>::PointGet(p,HORIZONTAL);

But if the interface used a free template function;

template<class PointConcept>
typename PointTraits<PointConcept>::CoordType
PointGet( PointConcept const& p, Orientation2D orient )
{
  return orient == HORIZONTAL ? p.x: p.y;
}

then I could just call:

  PointGet(p,HORIZONTAL);

which is arguably much better to a user.

Do *I*, the user, need to implement PointConceptMap and PointConcept?

> template <class T>
> class PointConcept : public T {
> public:
> static inline PointConcept& mimic(T& t) {
> return static_cast<PointConcept&>(t); }
>

So.

PointConcept& adapted_actual_data_ref = 
PointConcept<MyPoint>::mimic(actual_data);

is the way to introduce an instance of the adapter without copying 
actual_data.

OK, at least now I see how do you avoid copying.

Second note: This PointConcept class IS NOT a concept, is an adapter. At the 
very least it should not be called concept.

That is, in C++0x I won't be able to replace the definition with something 
like

template <class T>
concept PointConcept : ....

The reason is that the "role" if this design element is not to enforce that 
types(s) meets certain requirements but to adapt one very specific other 
type to meet them. In concept jargon, this is a *model*, not a concept.

> inline PointConcept() {}
>
> template <class T2>
> inline PointConcept& operator=(PointConcept<T2>& that) {
> set(HORIZONTAL, that.get(HORIZONTAL);
> return set(VERTICAL, that.get(VERTICAL); }
>
> Unit get(Orientation2D orient) const { return get_(orient); }
> PointConcept& set(Orientation2D orient, Unit value) {
> set_(orient, value); return *this; }
>

Nitpicking here, but, why is set() forwarding to set_() instead of doing 
this itself?

> template <class T>
> void foo(T& t) {
> PointConcept<T>& tref = PointConcept<T>::mimic(t);
> tref.set(HORIZONTAL, tref.get(VERTICAL);
> }
>

OK, so at the very least I can pass my point object as-is.
In some code posted my Gyuzsi (I think) the user itself was explicitely 
wrapping at the point of call.

OK, you showed us how the pattern works.

So, as a user I need to implement PointConceptMap<> and PointConcept<>, is 
that right?

That seems like a lot of unnecesary work to me just to call a library.

So, let me start with a concrete review now that I see some actual stuff.

Why do I need to implement both  PointConceptMap and PointConcept??

Couldn't foo() be implemented like this instead?

template <class T>
void foo(T& t)
{
  PointConceptMap<T>::PointSet(t
                                                   ,HORIZONTAL
                                                  ,PointConceptMap<T>::PointGet(t,VERTICAL)
}

That cuts down the adaptation burden on my side by half. It even gets rid of 
the uggliest half (the one with the upcast).

Add to that what I said about using free functions to leverage template 
argument deduction and you get:

template <class T>
void foo(T& t)
{
  PointSet(t,HORIZONTAL,PointGet(t,VERTICAL);
}

Best

Fernando