[gdal-dev] Review of r38344 - CPLConfigOptionSetter

[Even Rouault]

Kurt,

> 1) C char * string vrs std::string vrs CPLString

> I'm not a fan of CPLString, so I'm glad you didn't use that.  But all over
> the code I see the use of CPLString when a std::string would work just
> fine.  And if I remember correctly, I've seen you do this before.  Why
> didn't you use a CPLString here?

This header doesn't include cpl_string.h, so CPLString isn't available. And there might be a 
subtle difference of behaviour between NULL and empty string in the previous value of the 
config option, that would be less convenient to take into account with std::string / CPLString
> 
> 2) Why not put it all in the header so that the compiler can inline it all
> as it is so small?
> 
> 

I didn't see this performance critical to be really worth inlining.

Even


-- 
Spatialys - Geospatial professional services
http://www.spatialys.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20170515/63e0f41a/attachment.html>