[boost] [Review] GGL review starts today, November 5th

Subject: [boost] [Review] GGL review starts today, November 5th
From: Christophe Henry (christophe.j.henry_at_[hidden])
Date: 2009-11-22 16:23:13

I apologize in advance for not being able to bring a complete review
due to my lack of knowledge of the problem domain, but I hope to bring
a few elements.
I also apologize in advance for the unfair treatment to Luke that I
was not able to do the same about Polygon due to my lack of time at
the point of the Polygon review.

*What is your evaluation of the design?
The design looks clear enough so that I was able to understand about
what was being done.
Adding concept checking is a good idea.
It also surely is a modern generic design.
I found only 2 points to criticize:
- linestring has std::vector as basis class, and therefore it should
be clearly documented that this class is not made to be derived from.
- most or all of geometries classes have no virtual destructor and
thus this should be clearly documented or changed. Better even would
be to give the user choice through a policy.

*What is your evaluation of the implementation?
The code is clean and nice to read and I found it pretty easy to
understand (apart from the deep geometry stuff of course).

*What is your evaluation of the documentation?
The examples are nice to read, the rest of the doc is a bit light and
would probably not let me do more complicated stuff.
I'd love an example about every feature.

*What is your evaluation of the potential usefulness of the library?
For me quite big. I'd probably never need the advanced features but
the simple ones would solve my biggest problem, which is how to avoid
ever having to implement something like this myself, which I would
probably never manage anyway.
About the completeness of the library, I am not able to judge it and
trust the more experienced reviewers.

*Did you try to use the library? With what compiler? Did you have any problems?
I compiled several examples with VC9 + VC10 beta2.
Both compiled without warnings level 3. The VC10 code analyser found a
few points (reported to one the library writers), which I suppose are
minor.
Interestingly, the analyser did not find a bug in the linestring
example, which happens only with VC10. This bug needs fixing but must
be pretty simple to correct.

*How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
I spent a few hours reading and running the examples and checking the
analyser and compiler level 4 warnings.
I also spent 2 hours looking at the code.

*Are you knowledgeable about the problem domain?
Not at all. I'm simply looking for a library in case I ever need to do
something in this domain.

*Do you think the library should be accepted as a Boost library? Be
sure to say this explicitly so that your other comments don't obscure
your overall opinion.
YES, eventhough I wouldn't count my own opinion as a full vote due to
my lack of domain expertise.
I like:
- some nice features I understood like the linestring and polygon
- the fact that the code is clean (meaning lots of care has been spent
on it) and that the authors are responsive, which is a sign that the
library will be well-maintained.
- the examples

I like less:
- the documentation which gave me the frustrating feeling I missed
most of the cool stuff and needs some warning messages  for the
problems I cited above.

Best regards,

Christophe Henry