Subject: Re: [boost] Formal Review: Boost.Polygon starts today August 24, 2009
From: Thomas Klimpel (Thomas.Klimpel_at_[hidden])
Date: 2009-09-01 14:08:22
Simonson, Lucanus J wrote:
> Barend Gehrels wrote:
> > I didn't try everything, but I'm getting the feeling that the concepts
> > are not implemented as they should be. The file
> > polygon_set_concept.hpp, which should give an outline of the polygon
> > set concept, is dependant on polygon_set_data.hpp (which implements
> > that concept). That should be the other way round. Inside that
> > concept file, points are often copied into
> > a polygon_set_data temporary. That is not how concepts should be
> > implemented, and is very bad for performance.
> I have to copy the polygon edges into something so that I can sort and scan them.
> That thing is encapsulated by the polygon_set_data class.
I think what Bahrend Gehrels observed here is related to my remark "..., and the source files implementing the concepts got overloaded with algorithms (or algorithm dispatch code).".
It would be cleaner to only implement the interface of the polygon_set concept in polygon_set_concept.hpp and implement the algorithms (or dispatch code) that work on the polygon_set concept in their own source file. This approach will also scale better as you add more algorithms to the library. I think it is a good idea that the algorithms use polygon_set_data internally, but it would not be a good idea if the implementation of the interface of the polygon_set concept would make use of polygon_set_data.
To be honest, I really think that it would be a good idea to "slightly change the organization of the code into source files".
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk