<div dir="ltr">My preference (and not speaking for the gdal community) for C++ classes would be:<div><br></div><div>1. replace const char * -> const std::string &</div><div>2. replace CPLString -> std::string</div><div><br></div><div>std::string GetString(const std::string &soName, const std::string &soDefault = "") const;<br></div><div><br></div><div>This is where it would be good to get input from others.</div><div><br></div><div>I base the above on maximizing safety while trying to let the compiler do its best job at optimizing. Then in about 2022, we can see about std::string_view :(</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 5, 2018 at 12:26 PM, Dmitry Baryshnikov <span dir="ltr"><<a href="mailto:bishop.dev@gmail.com" target="_blank">bishop.dev@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<p>Hi Kurt,</p>
<p>Can you explain what should be done in PR?</p>
<p>Do you mean to replace all const char* to?<br>
</p>
<p>1. const char* -> const CPLString &</p>
<blockquote>
<p>const char *GetString(const char *pszName, const char
*pszDefault = "") const; -></p>
<p>CPLString GetString(const CPLString &soName, const
CPLString &soDefault = "") const;</p>
</blockquote>
<p>or<br>
</p>
<p>2. const char* -> const std::string &</p>
<blockquote>
<p>const char *GetString(const char *pszName, const char
*pszDefault = "") const; -></p>
<p>std::string GetString(const std::string &soName, const
std::string &soDefault = "") const;</p>
</blockquote>
<p>or?<br>
</p>
<pre class="m_1403670294590659513moz-signature" cols="72">Best regards,
Dmitry</pre>
<div class="m_1403670294590659513moz-cite-prefix">05.01.2018 18:54, Kurt Schwehr пишет:<br>
</div><div><div class="h5">
<blockquote type="cite">
<pre>+1 for wrapping the old C code in some cleaner abstractions!
But +10 for switching to a from the ground up C++ JSON library unless there
are clear reasons for a core C library (I don't think there are)
If we are talking about this kind of code, there are several things that
have bugged me in general about GDAL for a long time.
* Passing char *psz yada all over the place in pure C++ code. A const
std::string is usually not a noticeable expense and is a lot safer
* CPLString when std::string will do just fine. And we can write free
functions to operate on strings. I'm generally bothered by subclassing of
std::string as CPLString. After reading large amounts of C++ code, I think
it adds more confusion than it ever helps over having clean free
functions. Interop and analysis with CPLString's is no fun.
<a class="m_1403670294590659513moz-txt-link-freetext" href="https://stackoverflow.com/questions/6006860/why-should-one-not-derive-from-c-std-string-class" target="_blank">https://stackoverflow.com/<wbr>questions/6006860/why-should-<wbr>one-not-derive-from-c-std-<wbr>string-class</a>
-kurt
On Fri, Jan 5, 2018 at 7:44 AM, Sean Gillies <a class="m_1403670294590659513moz-txt-link-rfc2396E" href="mailto:sean@mapbox.com" target="_blank"><sean@mapbox.com></a> wrote:
</pre>
<blockquote type="cite">
<pre>Hi Dmitry,
I scanned the PR and it seems reasonable to me. I'm barely a C++
programmer at all and it's clear to me, more clear than before. That said,
I'm not a fan of wrapping things that could be replaced. Have you looked
into whether a more performant C++ JSON library could be used? I haven't
run the benchmarks, but json-c compares pretty poorly to others in
<a class="m_1403670294590659513moz-txt-link-freetext" href="https://github.com/miloyip/nativejson-benchmark" target="_blank">https://github.com/miloyip/<wbr>nativejson-benchmark</a>.
On Wed, Jan 3, 2018 at 12:04 PM, Dmitry Baryshnikov <a class="m_1403670294590659513moz-txt-link-rfc2396E" href="mailto:bishop.dev@gmail.com" target="_blank"><bishop.dev@gmail.com></a>
wrote:
</pre>
<blockquote type="cite">
<pre>Hi everybody,
Happy new year and lot of success in 2018!
I would like to discuss my pull request <a class="m_1403670294590659513moz-txt-link-freetext" href="https://github.com/OSGeo/gdal/" target="_blank">https://github.com/OSGeo/gdal/</a>
pull/282
I created a thin wrapper around the json-c library which wide using in
GDAL.
This is C++ interface which hides C memory management and provides nice
API. The web or disk json documents reading chunk by chunk with progress
indication also added.
In future, the json-c can be easily switch to something other without
breaking the existing code.
The CPLJSONDocument/CPLJSONObject/<wbr>CPLJSONArray usage examples can be
found in frmts/pds driver and c++ unit test in autotest/cpp/test_cpl.cpp.
Is this ready to merge into the trunk? Any objections?
Best regards,
Dmitry
______________________________<wbr>_________________
gdal-dev mailing list
<a class="m_1403670294590659513moz-txt-link-abbreviated" href="mailto:gdal-dev@lists.osgeo.org" target="_blank">gdal-dev@lists.osgeo.org</a>
<a class="m_1403670294590659513moz-txt-link-freetext" href="https://lists.osgeo.org/mailman/listinfo/gdal-dev" target="_blank">https://lists.osgeo.org/<wbr>mailman/listinfo/gdal-dev</a>
</pre>
</blockquote>
<pre>
--
Sean Gillies
______________________________<wbr>_________________
gdal-dev mailing list
<a class="m_1403670294590659513moz-txt-link-abbreviated" href="mailto:gdal-dev@lists.osgeo.org" target="_blank">gdal-dev@lists.osgeo.org</a>
<a class="m_1403670294590659513moz-txt-link-freetext" href="https://lists.osgeo.org/mailman/listinfo/gdal-dev" target="_blank">https://lists.osgeo.org/<wbr>mailman/listinfo/gdal-dev</a>
</pre>
</blockquote>
<pre>
</pre>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">--<div><a href="http://schwehr.org" target="_blank">http://schwehr.org</a></div></div>
</div>