Re: [boost] peer review

From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2008-04-29 16:50:27

>>2. Camel back capitalization and class names capitalized.
>>This should be changed for consistency with the rest of Boost. 

>This should be changed for consistency with the rest of Boost.

Clearly, and if an extensive rewrite is undertaken that will give me the
opportunity to expunge capitalization.

>>4. No multiple inclusion protection except on gtl.h. 

>I definitely prefer templates.  At the very least, I want versions of 
>the individual headers
>that are safe to include directly so that I don't have to include all
of 
>gtl.  The unsafe versions
>of the headers should probably go in a separate directory.

Joel also expressed the opinion that generic programming could allow the
library to use floating point or integer arithmetic for coordinates.  I
remain skeptical of that.  Also, because the coordinate type template
parameter would go on practically everything, I fear it would become
onerous to the user (and me.)  This isn't CGAL where the whole point is
to vary the coordinate data type to do research on numerical types.  One
thing I have considered is to cluster the header files into useful sub
chunks, such as 2D only basic types, manhattan only algorithms, and
45-degree geometry algorithms and polygon types.  This way the user can
easily choose the subset of the library they are interested in by
including the chunks they need.  I suppose hiding the other headers
would make sense once the documentation is sufficient.

>>13. Class consistency.  If a class has a member function with a
certain 
>>name, and it makes sense for another class to have a member function 
>>that could be named the same thing those names should be the same.

>This important for more than just ease of use.  It is absolutely 
>essential for generic code that
>needs to process rectangles and rectangular prisms polymorphically.

I generally don't like to place a requirement on the template parameter
that it provide specific functions, and instead prefer to go through
adaptors, but I do see your point.  In my work, I rarely have the
freedom to modify a class in legacy code to conform to the requirements
set forth in a generic library I want to use (or write.)

One of the things I like about my current design is that it allows the
function prototype to disambiguate for the compiler (and the user) what
concepts the arguments are supposed to model.
For example:
        template <class T>
        template <class T2>
        bool RectangleImpl<T>::contains(const RectangleImpl<T2>&
rectangle);
requires the T and T2 both provide RectangleInterface adaptors.
(Obviously Impl is a mis-nomer and would be changed, but if I throw away
the unorthodox design pattern then it doesn't much matter.)
I also like that is isn't ambiguous to the user which rectangle is doing
the containing vs:
        template <class T, class T2>
        bool constains(const T& containing_rectangle, 
                                const T& contained_rectangle);
where the user will remember that there is a function that takes two
parameters, but have to check the header file or documentation to remind
themselves what the order means, since it is somewhat arbitrary.  I
would need to come up with a convention for ordering and be consistent.

The other unfortunate thing that happens is overloading of free
functions becomes problematic when types are generically polymorphic:
        template <class T, class T2>
        bool constains(const T& containing_prism, 
                                const T& contained_prism);
because a prism should provide both rectangle and prism adaptors and two
prisms should satisfy both functions the result in a compiler error even
when enable_if is used.  If we could induce it to compile, the user is
left with no way to call the rectangle version on two prisms whereas
with my library they would currently just do:
        bool containment = prism.mimicRectangle().contains(prism2);
to view the prism as a rectangle to get access to the rectangle version
of contains.

The only solutions I can see are to embed the conceptual types into the
function name such as rectangle_contains(a, b) and prism_contains(a, b)
or better still, make them into namespaces:
        namespace rectangle {
                template <class T, class T2>
                bool constains(const T& containing_rectangle, 
                                const T& contained_rectangle);	
        }
        namespace prism {
                template <class T, class T2>
                bool constains(const T& containing_prism, 
                                const T& contained_prism);
        }
which is workable.

I'm looking for the boost community's feedback on what approach to
rewriting the library they find preferable, and a sort of final decision
on whether the generic inheritance/static_cast/mimicry design-pattern I
came up with is unacceptable for acceptance into boost, implying that a
complete rewrite is truly needed.  Is the requirement that code never do
anything the standard doesn't guarantee is safe, or is the requirement
that code be portable and functional?  Are there other considerations
that make what I'm doing objectionable?

Thanks,
Luke