Re: [boost] Formal Review: Boost.Polygon

Subject: Re: [boost] Formal Review: Boost.Polygon
From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2009-09-15 13:28:44

Fernando Cacciola wrote:
> Hi Luke,
>>> 
>>> ** The polygon_data type should be mutable (i.e. support things like
>>> push_back()), or a mutable version should be provided, or
>>> std::vector/list<point_data> should work with minimal traits
>>> specialisation.
>> 
>> The example custom_polygon.cpp shows the traits for a list<CPoint>. 
>> I could make all containers of points models of polygons_concept by
>> default, but I wouldn't know which polygon concept the user prefers.
>> 
> A mere container of points should be a model of the most general
> polygon concept (which is polygon_concept)

This would prevent a user from defining it to be (for example) a polygon_90_concept in their application, forcing them to declare a different type to be their model of polygon_90_concept.  I have the example that shows how to make a container a model of polygon_concept.  That should be enough help for users who wish to do so.  Personally, I would not want a vector of points to automatically be a model of polygon because a vector of points is not a polygon most of the time, and in my view making it so would not be type safe.  If I felt that vector of points should be a model of polygon my polygon_data type would simply be a vector and not a class encapsulating a vector.

>>> I also found that polygon_data is specialised by the coordinate
>>> type, not the point type, so anyone using their own point type will
>>> have to provide their own polygon type. Is there some reason why I
>>> can't have polygon_data<my_point>?  Again, making it easier to use
>>> std::container<my_point> would help.
>> 
>> Point type is a trait.
>> 
> Can you elaborate in your response?
> 
> If I have
> 
> struct my_point { float x,y; }
> 
> How do I create a "polygon_data" that uses that point type?
> 
> AFAICT that is just not possible.

That is true.  My polygon_data types are not parameterized on point type.  If you want to use your own point type you will have to use your own polygon type.  There is no particular reason that polygon_data is not parameterized on point type, I was trying to keep its implementation simple.  I use my own point type internally in many instances, and there may be a dependency in the implementation details of some algorithm in which I know the polygon type is my own that I assume its point type is my own also.

>>> ** extents() and center() should return their results, rather than
>>> using an output reference.
>> 
>> Out parameters and equational reasoning have been discussed
>> elsewhere in the review. 
>> 
> Yes, but I'm not sure what is your position regarding that.
> Can you clarify it?

I agree that it is better to have these functions return their result rather than have an input/output parameter.  Originally I implemented each as returning my rectangle type and my point type respectively.  I changed them to allow the user to get the output as their own data type.  I could change them further to make them return an object that auto-casts to T if T is a model of rectangle or point concept.  I prefer not to add generic casting operators to my point and rectangle types because this would lead to confusing syntax errors, so I would use some sort of expression object as suggested when this issue was previously discussed.  This is a change I am perfectly willing to make if required.

Thanks,
Luke