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

Subject: Re: [boost] Formal Review: Boost.Polygon starts today August 24, 2009
From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2009-09-01 14:46:07

Thomas Klimpel wrote:
> 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". 

I agree that the presence of implementation details of the definitions of functions at the same place where they are declared makes the header files harder to read.  Are you suggesting that I separate the declarations of functions from their defintions and place the defintions in the detail directory?  Doing so would double the effort required to maintain the interfaces.  Most boost libraries implemented as header only do not seperate the declarations from the defintions.  I favor the idea in principle, however.

As to the dependency of the concept on polygon_set_data, it is not the interface, but the implementation that depends on polygon_set_data.  In order for the interface to be dependent on polygon_set_data the interface would need to refer to polygon_set_data in the declarations of functions, using it in the definition of the function is not a dependency of the interface, per se, but an implementation detail.  I don't believe that polygon_set_data is referenced anywhere in the declarations of concept functions.

Thanks,
Luke