[gdal-dev] CPLJSONDocument

[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
>
>
>
>


-- 
--
http://schwehr.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20180105/ea692007/attachment-0001.html>