read_wkt and write_wkt

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

read_wkt and write_wkt

Barend Gehrels
Hi,

Some things about wkt:

The read_wkt function currently returns true/false if it is successfully
or not, while inside lexical_cast<> is used which might throw an
exception. I would like to change this by returning void and always
throw an exception if it is not successful. Is this OK for everyone?

This exception is already defined as struct read_wkt_exception : public
geometry_exception, we need to move that geometry_exception to core. Is
that OK ?

Boost defines an "exception/exception.hpp" which is not used by boost
libraries. At least not by e.g. spirit. So I derive from std::exception,
like most boost libraries do, is that OK?

Finally the wkt is streamed like one of:
(G geometry where G is a template parameter or type)
1) cout << wkt<G>(geometry) << endl // needs <G>
2) cout << make_wkt(geometry) << endl // same but does not need <G>
3) cout << geometry << endl // only if stream_wkt.hpp is included

About 2)
The "make_" prefix is normally reserved for object generators. This is
one but actually it is, more strictly, generating an manipulator (which
is an object). Is the "make_" prefix appropriate here? I thought I asked
this earlier but cannot find that mail so probably I didn't.
Alternatively it would be:
1) cout << wkt_manip<G>(geometry) << endl
2) cout << wkt(geometry) << end // because this will be most widely used

About 3)
The last one is prone to errors if geometry is not a geometry or not a
tag. It is not living in namespace ggl to enable the library to stream
custom geometries which are living outside the namespace ggl. So it is
not in any namespace. Bruno recently mailed me that this behaviour will
never be accepted by Boost so we have to re-add the namespace there, or
remove it at all.

This is also for the upcoming WKB, and myself am writing VEShapes for a
project (Virtual Earth shapes), so we can harmonize it.

Regards, Barend






Reply | Threaded
Open this post in threaded view
|

read_wkt and write_wkt

Bruno Lalande
Hi,

> The read_wkt function currently returns true/false if it is successfully or
> not, while inside lexical_cast<> is used which might throw an exception. I
> would like to change this by returning void and always throw an exception if
> it is not successful. Is this OK for everyone?
>
> This exception is already defined as struct read_wkt_exception : public
> geometry_exception, we need to move that geometry_exception to core. Is that
> OK ?

Both points sound fine to me.

> Boost defines an "exception/exception.hpp" which is not used by boost
> libraries. At least not by e.g. spirit. So I derive from std::exception,
> like most boost libraries do, is that OK?

Boost folks usually like to avoid intra-dependencies, so I guess they
don't have the intention to use Boost.Exception too much inside Boost.
I propose to leave it like that for now. The second reason is that we
don't know this library much for now. We can always integrate it in
the future if we know it better and we realize it can bring us
something.

> About 2)
> The "make_" prefix is normally reserved for object generators. This is one
> but actually it is, more strictly, generating an manipulator (which is an
> object). Is the "make_" prefix appropriate here? I thought I asked this
> earlier but cannot find that mail so probably I didn't. Alternatively it
> would be:
> 1) cout << wkt_manip<G>(geometry) << endl
> 2) cout << wkt(geometry) << end // because this will be most widely used

I'd say that the make_prefix is used:
- when the function generates an object whose constructor would
otherwise need one or several template parameters
AND
- when the object is known by the user and usually explicitly
manipulated by him (e.g. std::pair).

Am I right to say that the wkt<> class is rather a "helper" class
whose only purpose is to pass a geometry to a stream, and will never
be used explicitly by the user for any other purpose? If it's the case
then we should forget the make_ prefix and stick to wkt(geometry).

> About 3)
> The last one is prone to errors if geometry is not a geometry or not a tag.
> It is not living in namespace ggl to enable the library to stream custom
> geometries which are living outside the namespace ggl. So it is not in any
> namespace. Bruno recently mailed me that this behaviour will never be
> accepted by Boost so we have to re-add the namespace there, or remove it at
> all.

I really don't know what's allowed and what's not in this regard. We
really should have a discussion on the Boost list about "is it allowed
to enable operator<< for any class satisfying a given concept" and if
yes, what are the rules to follow.

Regards
Bruno
Reply | Threaded
Open this post in threaded view
|

read_wkt and write_wkt

Barend Gehrels



>  
>> The read_wkt function currently returns true/false if it is successfully or
>> not, while inside lexical_cast<> is used which might throw an exception. I
>> would like to change this by returning void and always throw an exception if
>> it is not successful. Is this OK for everyone?
>>
>> This exception is already defined as struct read_wkt_exception : public
>> geometry_exception, we need to move that geometry_exception to core. Is that
>> OK ?
>>    
>
> Both points sound fine to me.
>  
OK, I'll change it then.

>  
>> Boost defines an "exception/exception.hpp" which is not used by boost
>> libraries. At least not by e.g. spirit. So I derive from std::exception,
>> like most boost libraries do, is that OK?
>>    
>
> Boost folks usually like to avoid intra-dependencies, so I guess they
> don't have the intention to use Boost.Exception too much inside Boost.
> I propose to leave it like that for now. The second reason is that we
> don't know this library much for now. We can always integrate it in
> the future if we know it better and we realize it can bring us
> something.
>  
OK, makes sense. Indeed it is written somewhere that boost libraries
should not use other boost libraries. I think in general this is an old
guideline, or applicable to specific libraries. We're using many things
of Boost which are specific for library programmers (remove_const) or
very handy (ranges).

>  
>> About 2)
>> The "make_" prefix is normally reserved for object generators. This is one
>> but actually it is, more strictly, generating an manipulator (which is an
>> object). Is the "make_" prefix appropriate here? I thought I asked this
>> earlier but cannot find that mail so probably I didn't. Alternatively it
>> would be:
>> 1) cout << wkt_manip<G>(geometry) << endl
>> 2) cout << wkt(geometry) << end // because this will be most widely used
>>    
>
> I'd say that the make_prefix is used:
> - when the function generates an object whose constructor would
> otherwise need one or several template parameters
> AND
> - when the object is known by the user and usually explicitly
> manipulated by him (e.g. std::pair).
>
> Am I right to say that the wkt<> class is rather a "helper" class
> whose only purpose is to pass a geometry to a stream, and will never
> be used explicitly by the user for any other purpose? If it's the case
> then we should forget the make_ prefix and stick to wkt(geometry).
>  
OK, makes also sense. I already did the veshape like this. So changing
the wkt is a small job, it is slightly more work to update the samples.
But it looks better indeed.

>  
>> About 3)
>> The last one is prone to errors if geometry is not a geometry or not a tag.
>> It is not living in namespace ggl to enable the library to stream custom
>> geometries which are living outside the namespace ggl. So it is not in any
>> namespace. Bruno recently mailed me that this behaviour will never be
>> accepted by Boost so we have to re-add the namespace there, or remove it at
>> all.
>>    
>
> I really don't know what's allowed and what's not in this regard. We
> really should have a discussion on the Boost list about "is it allowed
> to enable operator<< for any class satisfying a given concept" and if
> yes, what are the rules to follow.
>
>  
OK, the problem is actually also that it tries to stream everything not
streamable and then complains that it is not satisfying the concept.
I'll have a look again, didn't touch this part for months.

Regards, Barend



-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090428/79894348/attachment-0001.html