<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi,</p>
<p>I changed CPLJSONObject/CPLJSONArray methods string parameters
to std::string and merged last trunk changes.</p>
<p> Still leave 3 methods with const char* to simplify code because
<span style="color: rgb(36, 41, 46); font-family: -apple-system,
system-ui, "Segoe UI", Helvetica, Arial, sans-serif,
"Apple Color Emoji", "Segoe UI Emoji",
"Segoe UI Symbol"; font-size: 14px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps:
normal; font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255, 255,
255); text-decoration-style: initial; text-decoration-color:
initial; display: inline !important; float: none;">according to
standard conversion sequence, methods with input (const char*,
const char*) parameters executes method with signature
(std:;string&, bool):</span></p>
<blockquote>
<p><span style="color: rgb(36, 41, 46); font-family:
-apple-system, system-ui, "Segoe UI", Helvetica,
Arial, sans-serif, "Apple Color Emoji", "Segoe
UI Emoji", "Segoe UI Symbol"; font-size: 14px;
font-style: normal; font-variant-ligatures: normal;
font-variant-caps: normal; font-weight: 400; letter-spacing:
normal; orphans: 2; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: 2;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;">Add("key", "value") -->
Add(std::string osName, bool bValue)</span><br>
<span style="color: rgb(36, 41, 46); font-family: -apple-system,
system-ui, "Segoe UI", Helvetica, Arial, sans-serif,
"Apple Color Emoji", "Segoe UI Emoji",
"Segoe UI Symbol"; font-size: 14px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps:
normal; font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255,
255, 255); text-decoration-style: initial;
text-decoration-color: initial; display: inline !important;
float: none;"><span style="color: rgb(36, 41, 46);
font-family: -apple-system, system-ui, "Segoe UI",
Helvetica, Arial, sans-serif, "Apple Color Emoji",
"Segoe UI Emoji", "Segoe UI Symbol";
font-size: 14px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width:
0px; background-color: rgb(255, 255, 255);
text-decoration-style: initial; text-decoration-color:
initial; display: inline !important; float: none;">Set("key",
"value") --> Set(std::string osName, bool bValue)</span></span></p>
</blockquote>
See notes in pull request
(<a class="moz-txt-link-freetext" href="https://github.com/OSGeo/gdal/pull/282#issuecomment-355766795">https://github.com/OSGeo/gdal/pull/282#issuecomment-355766795</a>). <br>
I'm ready to merge current pull request into the trunk.<br>
<pre class="moz-signature" cols="72">Best regards,
Dmitry</pre>
<div class="moz-cite-prefix">05.01.2018 23:43, Kurt Schwehr пишет:<br>
</div>
<blockquote type="cite"
cite="mid:CACmBxyv_Ffmu2NbFxHvRfXWyNbHU1Zgb+W8BHOsNSsMzMZ413g@mail.gmail.com">
<pre wrap="">My preference (and not speaking for the gdal community) for C++ classes
would be:
1. replace const char * -> const std::string &
2. replace CPLString -> std::string
std::string GetString(const std::string &soName, const std::string
&soDefault = "") const;
This is where it would be good to get input from others.
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 :(
On Fri, Jan 5, 2018 at 12:26 PM, Dmitry Baryshnikov <a class="moz-txt-link-rfc2396E" href="mailto:bishop.dev@gmail.com"><bishop.dev@gmail.com></a>
wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Kurt,
Can you explain what should be done in PR?
Do you mean to replace all const char* to?
1. const char* -> const CPLString &
const char *GetString(const char *pszName, const char *pszDefault = "")
const; ->
CPLString GetString(const CPLString &soName, const CPLString &soDefault =
"") const;
or
2. const char* -> const std::string &
const char *GetString(const char *pszName, const char *pszDefault = "")
const; ->
std::string GetString(const std::string &soName, const std::string
&soDefault = "") const;
or?
Best regards,
Dmitry
05.01.2018 18:54, Kurt Schwehr пишет:
+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="moz-txt-link-freetext" href="https://stackoverflow.com/questions/6006860/why-should-one-not-derive-from-c-std-string-class">https://stackoverflow.com/questions/6006860/why-should-one-not-derive-from-c-std-string-class</a>
-kurt
On Fri, Jan 5, 2018 at 7:44 AM, Sean Gillies <a class="moz-txt-link-rfc2396E" href="mailto:sean@mapbox.com"><sean@mapbox.com></a> <a class="moz-txt-link-rfc2396E" href="mailto:sean@mapbox.com"><sean@mapbox.com></a> wrote:
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 inhttps://github.com/miloyip/nativejson-benchmark.
On Wed, Jan 3, 2018 at 12:04 PM, Dmitry Baryshnikov <a class="moz-txt-link-rfc2396E" href="mailto:bishop.dev@gmail.com"><bishop.dev@gmail.com></a> <a class="moz-txt-link-rfc2396E" href="mailto:bishop.dev@gmail.com"><bishop.dev@gmail.com></a>
wrote:
Hi everybody,
Happy new year and lot of success in 2018!
I would like to discuss my pull request <a class="moz-txt-link-freetext" href="https://github.com/OSGeo/gdal/">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/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
_______________________________________________
gdal-dev mailing <a class="moz-txt-link-abbreviated" href="mailto:listgdal-dev@lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev">listgdal-dev@lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev</a>
--
Sean Gillies
_______________________________________________
gdal-dev mailing <a class="moz-txt-link-abbreviated" href="mailto:listgdal-dev@lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev">listgdal-dev@lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev</a>
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>