Subject: Re: [boost] [Review] We're halfway through the GGL review
From: Fabio Fracassi (f.fracassi_at_[hidden])
Date: 2009-11-15 14:48:19
Barend Gehrels wrote:
> Hi Fabio,
>
> Thanks for your interest and for reviewing our library!
>
>
>>
>> polygon p1 = ...;
>> polygon p2 = ...;
>>
>> polygon_set ps1 = make_union(p1, p2);
>> polygon_set ps2;
>> intersection(ps1, p2, ps2);
> The polygon_set is implemented as a multi-polygon within GGL (and indeed
> you can call it polygon_set, if it fulfils the concept). We didn't merge
> the Multi* concepts into the generic algorithms, in other words, we
> don't require anyone to use a multi* (of course algorithms can be used
> for multi*).
Ok I read about multi* in the docu, but didn't make the connection.
I also find the ggl/multi/... includes somewhat surprising.
Maybe you can use them in one of the examples, to make things clearer.
>
> Therefore, union and intersection work with output iterators. So:
> union_inserter(p1, p2, std::back_inserter(ps1));
Hm, I tried:
#include <boost/ggl/ggl.hpp>
#include <boost/ggl/geometries/cartesian3d.hpp>
#include <boost/ggl/multi/geometries/multi_polygon.hpp>
#include <boost/ggl/algorithms/union.hpp>
...
typedef multi_polygon<polygon_3d> polygon_set_3d;
polygon_set_3d ps;
polygon_3d poly1;
polygon_3d poly2;
union_inserter(poly1, poly2, std::back_inserter(ps));
but got this error:
error: no matching function for call to
'union_inserter(ggl::polygon_3d&, ggl::polygon_3d&,
std::back_insert_iterator<ggl::multi_polygon<ggl::polygon<ggl::point<double,
3ul, ggl::cs::cartesian>, std::vector, std::vector, true,
std::allocator, std::allocator>, std::vector, std::allocator> >)'
Am I doing something wrong?
>
> It is possible that there will be a "intersection" and "union_" as well,
> detecting if ps1 is a multi-polygon and automatically inserting. But
> that is more a sort of synonym, similar way to do things.
Yes, I know just a bit of syntactic sugar, but IMO it makes the
interface much easier to use.
Besides I would probably not call it union_ but geometric_union or some
such, but thats just personal preference.
>
> The make_union would require an r-value and if C++0x is there, it can be
> implemented without copying the whole resultset. Indeed, it fits into
> the design and naming conventions.
Of course, still I like this syntax best, as it is most natural.
>
>> On the whole I am very pleased with the documentation, its easy to
>> read and reasonably complete.
>> I have a general problem with doxygen generated docu with regard to
>> generic libraries.[...]
> I understand. Several people have addressed this so we will certainly
> improve the documentation. Thanks for your structure proposal.
>
Fine, while I was further investigating ggl docu I found another wish:
For each class/function add a short usage example including all relevant
includes (something short like the snipplet above)
This makes it immediately clear how things work together.
Regards,
Fabio