Boost logo

Geometry :

Subject: Re: [geometry] extensions
From: Barend Gehrels (barend_at_[hidden])
Date: 2012-02-07 14:02:54


Hi Kris,

Thanks for your patches!

On 7-2-2012 17:51, Krzysztof Czainski wrote:
> Hello again,
>
> I present a few notes with patches after some further experience with
> Boost.Geometry.
>
> First of all, I think I read in boost guidelines, that it is
> discouraged that a class and namespace enclosing it have the same
> name. And in such cases, the convention is for the namespace to be
> named in plural. So I suggest to rename the namespace
> boost::geometry::projection to projections. I did not prepare a patch
> for that ;-)

Yes, I remember the discussion about this recently (think last year).
And the guidelines will be extended after that, IIRC. But we were
already developing before...

So we have:
namespace strategy
namespace model

both singular, and we have also:

namespace traits
namespace detail::iterators (but this is not for the end user)

in plural.

So not consistent everywhere. The names of the folders are in plural
(all where relevant)

Then about projection, indeed we have a class projection in namespace
projection. I forgot that, strange actually because that can give
problems indeed. So yes, agree with your proposal. It is a breaking
change in a way but in extensions we still accept that (in Trunk as well
but only if there are really very good reasons).

Note that all real projection implementations are converted
automatically so yes, better to not provide a patch...

>
> Secondly, I wanted to remove the dependency on boost/lexical_cast.hpp.
> I noticed that BOOST_GEOMETRY_NO_LEXICAL_CAST is already used in some
> parts of Boost.Geometry, so I just added the use of this to other
> places, and no_lexical_cast.patch contains the changes.

OK, good idea.

>
> Next, I work with a Texas Instruments compiler, which has this defect,
> that static local variables of inline functions are not resolved to a
> single instance, and it warns about it. So I made two minor changes:
> static_inline.patch. I think they wouldn't hurt in any case, but I
> could be wrong here.

Don't think so either, so no problem for me.

>
> And lastly, I encountered a macro problem - another defect of my
> compiler - it has a #define cosl cos, so there are problems with local
> variables named cosl. cosl.patch.

OK, so that one has to be included in the conversion program... But that
will be no problem.

So I will apply your patches, at least the idea contained in them.

Last week was still busy with the release and some other things, and
coming weekend is the last before the deadline, so it will take a week
or so.

> It should be fairly easy to implement such a function, and I
> am willing to do it, if you guys think it's a good idea ;-)
>
>
> Yes, this sounds as a good idea! I will look into more detail to
> this (hope doing it soon - more and more things come on the list
> last days)
>
>
> I have also prepared functions new_procjection(par) and
> make_shared_projection(par), but due to lack of time right now, I'll
> present them within a few days.

Perfect! Yes, better to provide it - I did not look at the projections
in between, but will do with the other actions and your possible patch.

Regards, Barend



Geometry list run by mateusz at loskot.net