Re: [boost] Formal Review: Boost.Polygon starts today August 24, 2009

Subject: Re: [boost] Formal Review: Boost.Polygon starts today August 24, 2009
From: Joachim Faulhaber (afojgo_at_[hidden])
Date: 2009-09-02 13:33:47

2009/8/24 Fernando Cacciola <fernando.cacciola_at_[hidden]>:
> The formal review of the Boost.Polygon library by Lucanus Simonson starts
> today, August 24, 2009 and will finish September 2, 2009.
>
> ---------------------------------------------------
>
> Please always state in your review, whether you think the library should be
> accepted as a Boost library!
>

YES, boost::polygon should be accepted as a boost library.

After all the objections being put on the table during the
review process and after some difficulties experienced
during a few of my own experiments my yes vote is
not as big as in the beginning but still a yes.

* The design and overall appearance of the library
is characterized by clarity, minimality and elegance.

* The library comes from a demanding industrial background
and runs, if I understood right, already successfully under
real word conditions.

* The library has a defined scope. Within this scope it is
well designed and convinces me by a clear and
intuitive interface and a smart method to connect client
data types.

* When big companies open up their sourcecode on
the initiative of young developers striving for genericity
and more universal use of their software I think this should
not lightly be discouraged.
Beyond technical objections I'd like to strengthen
such culture and to encourage such initiatives.

>
> Additionally please consider giving feedback on the following general
> topics:
>
> - What is your evaluation of the design?

The design minimizes dependencies. Class templates
are small and semantics is coded into a system of
free functions which makes the library lean and
flexible so the different types can be combined
in many beneficial ways.

The system of overloads allows for the combination
of certain types to select the more efficient algorithms.

Whether the system of meta predicates produces all
desired overloads with the different compilers using
SFINAE is difficult to judge, at least for me.

> - What is your evaluation of the implementation?
I did not check any algorithms.

* A few things should be done:

There are literals used instead of named constants.

e.g.
polygon_set_concept 386:
return self_assignment_boolean_op
  <geometry_type_1, geometry_type_2, 3>(lvalue, rvalue);
literals '3' should be a named constant, it codes the
semantics of this operation's call.

Some identifiers are rather cryptic violating
boost naming conventions...
e.g.
polygon_set_concept 347:
  struct yes_ps_ose : gtl_yes {};

... and obscuring the meaning of the enable_if
statements they are used in.

c style casts as has been discussed already
are used frequently throughout the whole
implementation. IMO they should be all
replaced.

There are (still) warnings when compiling with
msvc. These should be reduced as far as
this is possible with reasonable effort.

> - What is your evaluation of the documentation?

The documentation is lean and minimal.
The examples are easy to understand and convincing.

Sometimes a little more information and redundancy
would be helpful.

There is a section missing that deals with the
requirements that client classes have to adjust to.
There should be a warning that client polygon set
classes must not implement own self assign
operators.

> - What is your evaluation of the potential usefulness of the library?

I think the scope of the library as providing basic
algos in planar geometry for fields like VLSI, CAD
is large enough. Also I do not see a problem
of coexisting boost::polygon and boost::GGL, *because*
of their different scopes.

I do not think every library, if well designed
and implemented correctly has to have an
intergalactic scope to be considered valuable.

> - Did you try to use the library?  With what compiler?  Did you have any
> problems?

I used the library with msvc-9 creating a few toy and test
programs and combining it with own types and tools.
I had some problems with operator overloading.
These have been discussed in other threads.

> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

Not complete but substantial readings of the docs.
Some browsing of code.
Code usage experiments for several hours.

> - Are you knowledgeable about the problem domain?
>

I'm not an expert in computational geometry but a seasoned
developer in fields of generic programming.

Best regards
Joachim Faulhaber