[geos-devel] SegmentString modeling

[Mateusz Łoskot]

strk at refractions.net wrote:
> On Thu, Feb 16, 2006 at 07:12:32PM +0100, Mateusz Å?oskot wrote:
>>strk at refractions.net wrote:
>>
>>>Are you suggesting SegmentString should take ownership
>>>of the passed CoordinateSequence ?
>>>This is not how it currently works. Not that I like current
>>>behaviour, but I'm trying to understand your suggestion.
>>>
>>>In the examples you show B takes ownership of B*, things
>>>get simpler in that case. But when B does not own C, changing
>>>C isn't the same as changing B.
>>
>>In my example B owns object assigned to B::c because it's simple example 
>>and my aim was to make it correct.
>>But in my idea ownership of object assigned to B::c is not important.
>>We are talking about const correctness what does not depend on objects 
>>ownership.
> 
> 
> I somehow disagree on this.
> If B does not own C*, changing C* should not be the
> same as changing B iself.


OK, I agree with small note: ownership depends on relation defined
between B and C. This relation is a matter of design.

>>How is responsible to destroy CoordinateSequence assigned to 
>>SegmentString in current solution?
>>As I see SegmentString's destructor does not do it.
>>So, if you will make SegmentString::pts member as non-const:
>>
>>CoordinateSequence *pts;
>>
>>it won't change the situation regarding ptr ownership at all.
> 
> 
> No, but it won't be able to assign a const pointer to it.

Yes, that may be the problem.
Another option, I assume SegmentString owns CoordinateSequence,
is to add constructor assigning CoordinateSequence by-copy.

SegmentString(CoordinateSequence const& newPts)
{
    // 1. Create new CoordianteSequence dynamically, assign to pts
    // 2. Make a copy oc newPts

    // pts will be deleted in SegmentString destructor
}

or keep two constructors taking pointers:

// This one will assign newPts to CoordinateSequence::pts
SegmentString(CoordinateSequence* newPts);

// This one will make a copy of newPts and assing
// new copy to CoordinateSequence::pts
SegmentString(const CoordinateSequence* newPts)

Note: Making changes related to const correctness in current stage of 
GEOS may be a big challange and may require to refactor huge amount of code:
http://parashift.com/c++-faq-lite/const-correctness.html#faq-18.3

> The problem can be seen in operation/Buffer/OffsetCurveSetBuilder.cpp.
> The addCurve(const CoordinateSequence* coord) function constructs
> a SegmentString passing it 'coord'. This is possible as far as
> SegmentString promises not to change 'coord' itself.
 > Indeed SegmentString won't change it, but then it won't provide
 > any method to get a non-const ref to let others change it.

Is this promise a matter of logic or because of that coordinate sequence 
is hold by const pointe?
const CoordinateSequence* pts;

> This (changing CoordinateSequence trhough SegmentString) would
> be required by the ScaledNoder class, which I'm about to port
> from JTS.

So, if I understand it correctly, you need to make currently read-only 
SegmentString::pts to be changable. Right?
But why SegmentString::pts can not be read-write but accessed only 
through read-only interface (member functions returning/taking const 
reference or pointer) in operation/Buffer/OffsetCurveSetBuilder.cpp
and through read-write interface in ScaledNode.

I'm still trying to point out that the context is really important so 
you don't have to make object const if the context will promise the 
const correctness.

> Currently, the CoordinateSequence ownership is:
> 	- of OffsetCurveBuilder (ptList)
> 	- of SegmentNodeList 
> 	- of Edge (in EdgeNodingValidator, unused actually)
> 
> I guess making SegmentString own the CoordinateSequence could
> be a viable solution for now.

Yes, that seems to be OK.

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