[Proj] C++ coding practices w.r.t object ownership

[Even Rouault]

> I think that the "CI_", "EX_" and "MD_" prefixes can be omitted. They
> duplicate namespaces/packages in C++/Java and recent ISO/TC211 policy
> (in my understanding) is to drop them in newer standards. For example
> "Datum" in ISO 19111:2018 was "CD_Datum" in ISO 19111:2007.

Yes, sounds reasonable

> 
> PositionalAccuracy (currently in "common" namespace) belong to
> "metadata" namespace too.

OK

> 
> Regarding the object ownership discussion, one more criterion that may
> be worth to consider is whether the proposed alternatives support cyclic
> references. For example ProjectedCRS.conversion references a Conversion
> object, which could in turn reference back the ProjectedCRS in its
> Conversion.targetCRS attribute. ISO 19111 allows to break this
> circularity by allowing Conversion.source/targetCRS to be null in such
> cases. But I find convenient to nevertheless provide the reference value
> when the language/framework support cyclic references since it makes
> easier to use a Conversion instance without carrying extra information
> about its context.

Yes, that's an annoying point I noticed yesterday when looking more closely at 
this, since cyclic references are unfriendly with shared pointers.

The solution is that CoordinateOperation stores sourceCRS and targetCRS as 
std::weak_ptr internally. In the sourceCRS() and targetCRS() getters, those 
weak pointers are converted to shared pointers with .lock() (the 
shared_pointer being then potentially null if the owning ProjectedCRS has been 
destroyed in between). So users of the API only see shared pointers. And add  
a documentation note of sourceCRS() and targetCRS() about the particular case 
of ProjectedCRS.derivingConversion.

Bonus point: when the CoordinateOperation is created in all other contexts 
than the derivingConversion of a ProjectedCRS, then CoordinateOperation can 
also have internal shared_ptr so as to make sure than the source and target 
CRS are kept alive, and the weak_ptr conversion to a shared_one always return 
a non null pointer.

Attached a POC (simplified but representative of the above situation) that 
works pretty well: no memory leak, and safe pointer usage.


> Circularity may also happen in ISO 19115 metadata,
> e.g. between Platform and Instrument classes.

OK. I don't think I'll need those ones. Enough classes for now !

Even

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testweak.cpp
Type: text/x-c++src
Size: 2387 bytes
Desc: not available
Url : http://lists.maptools.org/pipermail/proj/attachments/20180529/816dafd2/attachment.cpp