<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jul 25, 2020 at 9:41 AM Sandro Santilli <<a href="mailto:strk@kbt.io">strk@kbt.io</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat, Jul 25, 2020 at 07:12:08AM -0700, Martin Davis wrote:<br>
> Good observations, Sandro.  My thoughts are below.<br>
> <br>
> On Fri, Jul 24, 2020 at 8:01 AM Sandro Santilli <<a href="mailto:strk@kbt.io" target="_blank">strk@kbt.io</a>> wrote:<br>
> <br>
> ><br>
> > The methods defined for a Noder are:<br>
> ><br>
> >   virtual void computeNodes(std::vector<SegmentString*>* segStrings) = 0;<br>
> >   virtual std::vector<SegmentString*>* getNodedSubstrings() const = 0;<br>
> ><br>
> > What isn't clear is:<br>
> ><br>
> >   - Who owns the SegmentString passed to computeNodes ?<br>
> >   - Who owns the SegmentString returned by getNodedSubstrings ?<br>
> >   - Can the same SegmentString be present in both input and output<br>
> >     containers ?<br>
> ><br>
> <br>
> - Noders are a process, not a container.  As such, they should not own<br>
> anything.<br>
> -- The caller owns the NodedSegmentStrings passed in to the Noder.<br>
> -- The caller owns the SegmentStrings returned by getNodedSubstrings<br>
> <br>
> - getNodedSubstrings should always return new SegmentStrings, even if they<br>
> are just a copy of an input SegmentString (i.e. no nodes were found and<br>
> added to the original NodedSegmentString (This will make it easier to<br>
> handle the lifecycle of the inputs and outputs I think?)<br>
> <br>
> I'm not sure if JTS/GEOS obeys these semantics. In JTS it doesn't matter<br>
> much, but GEOS should be fixed to have this contract.<br>
<br>
For sure it should be fixed to DOCUMENT this contract.<br></blockquote><div> </div><div>Definitely. Using C++ language constructs, if possible. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I think current (the ones reachable from C-API) do respect it,<br>
which means that some of them have to DESTROY SegmentString objects<br>
that only exist temporary (think recursive noder) because those<br>
objects would never be returned back to caller, thus woule be left<br>
leaking.<br></blockquote><div><br></div><div>Well, yes.  The new noders (SnapRouningNoder and SnappingNoder) are not recursive, so don't have this problem.  The goal I think is to eliminate other kinds of noding, since they should be superseded.  But I realize that might have to be a long-term goal.</div><div><br></div><div>Anyway, if there are noders with that behaviour, don't they already handle their own memory?  Or are there leaks in old code?</div><div><br></div><div> </div></div></div>