[geos-devel] SegmentString modeling

[Mateusz Łoskot]

strk at refractions.net wrote:
> Need some help from C++ gurus.
> One of the new JTS classes expects to modify
> the Coordinates associated with a SegmentString.
> 
> So far the SegmentString kept a const-pointe
> to an externally provided CoordinateSequence.
> This means you can't modify a CoordinateSequence
> getting it's pointer from the SegmentString,
> even if the SegmentString itself is non-const
> (it's not the owner of the CoordinateSequence).

I've taken a look at GEOS sources and that's clear for me.

> Changing the interface to allow SegmentString
> to keep non-const pointers to CoordinateSequence
> instead, will break all promises of constness
> on which existing code relies.

IMHO it does not make sense saying storing data member by non-const 
pointer "will break all promises of constness".
You can store data member as private and non-const and you still can 
provide 100% of const correctness by encapsulation this data member.

IMO current design of collaboration of SegmentString and 
CoordinateString is wrong. SegmentString should store non-const pointer 
to CoordinateString *and* provide appropriate API which keep things 
correct from const point of view.
Keeping CoordinateSequence by const pointer is really hazardous because 
it will likely be changed/edited in some contexts.

Check STL sources and you will see that e.g. std::vector::bedin() and 
std::vector::end() are provided in two versions: const and non-const, so 
you can retrive vector elements by as read-only - by const reference or 
read-write by non-const reference depending on current context.

Check this piece of code:

#include <iostream>
using namespace std;

struct C
{
     int mx;
     int my;
     C() : mx(0), my(0) {}
};

class B
{
     C* c;  // Store C by non-const pointer
public:
     B(C* pc) : c(pc) {}
     ~B() { delete c; }

     // Const correctness provided by properly designed API
     C* get() { return c; }
     const C* get() const { return c; }
};

int main()
{
     // Create objects
     C* pc = new C();
     B b(pc);

     // Retrieve B::c by non-const pointer to edit B::c data members
     C* c1 = b.get(); // Here no constness is required
     cout << "0: " << c1->mx << "  " << c1->my << endl;
     c1->mx = 9;
     c1->my = 10;

     // Now, retrieve B::c by *const* pointer to only *read* B::c member
     const C* c2 = b.get(); // Here const correctness is provided
     cout << "1: " << c2->mx << "  " << c2->my << endl;

     // uncomment and you'll get compilation error
     //c2->mx = 10;  // c2 is read-only !!!
}


So, B::get() interface provide proper const correctness if any context 
will require it.

Certainly, in other classes, non-editable by design, you can provide 
read-only interface:

class ReadOnlyB
{
     C* c;  // Store C by non-const pointer
public:
     B(C* pc) : c(pc) {}
     ~B() { delete c; }

     // Const correctness provided by properly designed API
     const C* get() const { return c; }
};

> What I'd like to model is a SegmentString
> that will either store a const or a non-const
> pointer to CoordinateSequence depending on
> construction. Is it possible at all w/out 
> making it a templated class ?


I don't like it. Well working const correctness and encapsulation can be 
achived by properly modeled class interface but not by class internals.
Class internals should be separated from outside world by class 
interface and not its (internals) implementation, that's why you can 
hide data in class and provide proper interface. If you will need 
another behaviour e.g. read-write instead of read-only then you can add 
functions to class interface and if you need read-only behaviour of 
read-write class then you can wrap it behind read-only interface.

Cheers
-- 
Mateusz Łoskot
http://mateusz.loskot.net