<div dir="ltr"><div dir="ltr">Good observations, Sandro.  My thoughts are below.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 24, 2020 at 8:01 AM Sandro Santilli <<a href="mailto:strk@kbt.io">strk@kbt.io</a>> wrote:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<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></blockquote><div><br></div><div>- Noders are a process, not a container.  As such, they should not own anything.</div><div>-- The caller owns the NodedSegmentStrings passed in to the Noder.</div><div>-- The caller owns the SegmentStrings returned by getNodedSubstrings </div><div><br></div><div>- getNodedSubstrings should always return new SegmentStrings, even if they are just a copy of an input SegmentString (i.e. no nodes were found and added to the original NodedSegmentString (This will make it easier to handle the lifecycle of the inputs and outputs I think?)</div><div><br></div><div>I'm not sure if JTS/GEOS obeys these semantics. In JTS it doesn't matter much, but GEOS should be fixed to have this contract.</div><div><br></div><div>Furthermore:</div><div><br></div><div>The fact that getNodedSubstrings returns NodedSegmentStrings is an mistake caused by an unfortunate shortcut in JTS a long time ago.  In fact, almost all usage of getNodedSubstrings only requires a BasicSegmentString to be returned (which is a much simpler/smaller object).  Paul & I have discussed fixing this in JTS and GEOS, and will likely do so during or after the delivery of OverlayNG.  Also, the name of the method is then confusing, and will be changed as well (perhaps to getSubstrings or splitSubstrings or some such)</div></div></div>