[gdal-dev] CPLJSONDocument

[Dmitry Baryshnikov]

Hi,

I changed CPLJSONObject/CPLJSONArray  methods string parameters to 
std::string and merged last trunk changes.

Still leave 3 methods with const char* to simplify code because 
according to standard conversion sequence, methods with input (const 
char*, const char*) parameters executes method with signature 
(std:;string&, bool):

    Add("key", "value") --> Add(std::string osName, bool bValue)
    Set("key", "value") --> Set(std::string osName, bool bValue)

See notes in pull request 
(https://github.com/OSGeo/gdal/pull/282#issuecomment-355766795).
I'm ready to merge current pull request into the trunk.

Best regards,
     Dmitry

05.01.2018 23:43, Kurt Schwehr пишет:
> 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 <bishop.dev at gmail.com>
> wrote:
>
>> 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.
>> https://stackoverflow.com/questions/6006860/why-should-one-not-derive-from-c-std-string-class
>>
>> -kurt
>>
>> On Fri, Jan 5, 2018 at 7:44 AM, Sean Gillies <sean at mapbox.com> <sean at mapbox.com> 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 <bishop.dev at gmail.com> <bishop.dev at gmail.com>
>> wrote:
>>
>>
>> Hi everybody,
>>
>> Happy new year and lot of success in 2018!
>>
>> I would like to discuss my pull request https://github.com/OSGeo/gdal/
>> 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 listgdal-dev at lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev
>>
>>
>>
>>
>> --
>> Sean Gillies
>>
>> _______________________________________________
>> gdal-dev mailing listgdal-dev at lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev
>>
>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20180109/ba71b665/attachment.html>