[Proj] C++ coding practices w.r.t object ownership
Even Rouault
even.rouault at spatialys.com
Sun May 27 10:35:40 EST 2018
Hi,
I just wanted to give a very early preview of my prototyping of the ISO
191111:2018 class hiearchy, so we can discuss some coding practices (so
non-"cosmetic" aspects of coding), especially about fundamental conventions
deeply impacting the API, particularly regarding object ownership.
This is not material ready for detailed review: there are lots of TODOs, no
docs, mostly untested, doesn't do anything really useful yet, etc.
I've uploaded the Doxygen doc generated from the work in progress. Two useful
links to have an overview:
Diagram of class hiearchy:
http://even.rouault.free.fr/proj_cpp_api/html/inherits.html
Namespaces/packages and classes
http://even.rouault.free.fr/proj_cpp_api/html/annotated.html
Example of the CRS package (formatted with clang-format):
https://github.com/rouault/proj.4/blob/iso19111/src/crs.hh
https://github.com/rouault/proj.4/blob/iso19111/src/crs.cpp
Test program:
https://github.com/rouault/proj.4/blob/iso19111/test/test_cpp.cpp
A few principles I've adopted:
- done in a way such that memory leaks cannot happen (internal use of
unique_ptr)
- all private members are strictly hidden in the header file, by delegating to
a internal private struct in the .cpp file. This way we can add private
members without impacting the ABI
- from the GeoAPI design, I've kept the apparent design constraint of having
immutable objects once constructed.
- the getters generally return a pointer or const reference to the internal
structure members. Which means that those objects must not be used after their
belonging object has been destroyed
~~~~~~
There are different ways that could be used for object life-cycle management:
1) the one I took in this first sketch: we pass objects by const reference,
and methods/constructors that need to keep them, create a copy.
For example:
GeographicCRS createGeographicCRS(const PropertyMap& properties,
const GeodeticReferenceFrame& datum,
const EllipsoidalCS& cs);
will construct a GeographicCRS object that will duplicate datum and cs
internally.
2) another one we could imagine: use shared pointers
The above would become:
std::shared_ptr<GeographicCRS>
createGeographicCRS(const PropertyMap& properties,
std::shared_ptr<GeodeticReferenceFrame> datum,
std::shared_ptr<EllipsoidalCS> cs);
3) have ad-hoc conventions regarding pointer ownership. But we probably want
to avoid this as this is error-prone for API users (incluing PROJ internal
use)
~~~~~~
Comparing the 2 first approaches:
Copying approach:
- pros: no null pointer checking needed when acception a const& argument
- cons:
* can involve a lot of copying around when building complex object
hiearchies
* internally involves a kind of hack ( clone() method ) so that the
duplication correctly clones instances of derived classes. To be clear: let's
consider the situation where you have a constructor that accepts a Base& or
Base* argument, that you need to store as a class member. If you call it with
a Derived object, you want a instance of Derived to be cloned, not its Base-
only part.
shared_ptr approach:
- pros:
* reduced copying, avoid the cloning hack
* we could return shared_ptr objects in getters, making them super safe to
use (the return can be used after the belonging object has been destroyed).
- cons:
* null-pointer situations can happen and must be checked / documented
My feeling is that the shared_ptr approach could be better, although CRS
instanciation is probably not critical in most use cases. Of course shared_ptr
aren't completely free performance-wise, but I feel this would be more
efficient than deep copying of complex objects.
And, to save explicit null checking in situations where we don't want null
pointers, we could wrap the shared_ptr in a nn<> class [1] that guarantees
that the hold pointer is not null.
We would then have:
GeographicCRS_nnshptr
createGeographicCRS(const PropertyMap& properties,
GeodeticReferenceFrame_nnshptr datum,
EllipsoidalCS_nnshptr cs);
with using xxxx_nnshptr = nn_shared_ptr<xxxx>;
Thoughts ?
Even
[1] https://github.com/dropbox/nn/blob/master/nn.hpp
--
Spatialys - Geospatial professional services
http://www.spatialys.com
More information about the Proj
mailing list