Re: [review][JSON] Review of JSON starts today: Sept 14 - Sept 23

From: Rainer Deyke (rdeyke_at_[hidden])
Date: 2020-09-15 14:12:47

On 14.09.20 09:30, Pranam Lashkari via Boost wrote:
> Please provide in your review information you think is valuable to
> understand your choice to ACCEPT or REJECT including JSON as a
> Boost library. Please be explicit about your decision (ACCEPT or REJECT).
> 
>    - What is your evaluation of the design?

Most of it seems fine, but I do have some issues:

   - The choice of [u]int64_t seems arbitrary and restrictive.  It means 
that Boost.JSON will not use a 128 bit integer even where one is 
available, and it means that it cannot compile at all on implementations 
that don't provide int64_t.  It's good enough for my purposes, but I 
would like to see some discussion about this.  The json spec allows 
arbitrarily large integers.

   - On a similar note, the use of double restricts floating point 
accuracy even when a higher-precision type is available.  The json spec 
allows arbitrarily precise decimal values.

   - boost::json::value_to provides a single, clean way to extract 
values from json, but it also renders other parts of the library (e.g. 
number_cast) redundant except as an implementation detail.

   - boost::json::value_to provides a single, clean way to extract 
values from json, but it's syntactically long.  The same functionality 
in a member function of boost::json::value would be nicer to use.

   - The omission of binary serialization formats (CBOR et al) bothers 
me.  Not from a theoretical point of view, but because I have actual 
code that uses CBOR, and I won't be able to convert this code to 
Boost.JSON unless CBOR support is provided.  (Or I could write my own, 
but that rather defeats the point of using a library in the first place, 
especially if Neils Lohmann's library already provides CBOR support.)

>    - What is your evaluation of the implementation?

I didn't look at it.

>    - What is your evaluation of the documentation?

boost::json::value_to provides a single, clean way to extract values 
from json, but it's actually rather hard to find in the documentation. 
I was looking for a way to extract a std::string from a 
boost::json::value, so I looked at the documentation for 
boost::json::value and found as_string.  OK, that returns 
boost::json::string, which is not implicitly convertible to std::string 
(in C++14).  But it has an implicit conversion to string_view, which is 
an alias of boost::string_view, which doesn't appear to be documented 
anywhere but which (looking at the source code) has a member function 
to_string.  So I ended up with this code:

   boost::json::value v = "Hello world.";
   std::string s
     = static_cast<boost::json::string_view>(v.as_string()).to_string();

...which I think we can all agree is an abomination.  /Then/ I found out 
about boost::json::value_to, and replaced my code with this:

   std::string s = boost::json::value_to<std::string>(v);

Which is definitely nicer, though still not as nice as Niels Lohmann's code:

   std::string s = v.get<std::string>();

boost::json::value_to or its member function replacement should 
definitely be front and center to the documentation.

>    - What is your evaluation of the potential usefulness of the library?

A good json library is very useful for a large number of programs.  The 
questions isn't if Boost should have a json library, but if this is the 
json library that Boost should have.

>    - Did you try to use the library? With which compiler(s)? Did you have
> any problems?

I converted a small but non-trivial program from Lohmann's json to the 
proposed Boost.JSON, and compiled it with several different 
cross-compilers.  I did not encounter any problems compiling or running 
the program, although I did have to add Boost.Container to the set of 
linked libraries.

>    - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

A few hours of work, most of which was spent converting code from 
Lohmann's json library to the proposed Boost.JSON.

>    - Are you knowledgeable about the problem domain?

Yes.

 > Please be explicit about your decision (ACCEPT or REJECT).

For me, the ultimate question is if I would actually use this library, 
and my reluctant answer is "not its current state".  I'm basically 
satisfied with Lohmann's json library, which requires less verbosity to 
use and which provides CBOR support.  I can see the attraction of 
Boost.JSON's superior performance, and the attraction of incremental 
parsing and serialization, but for my usage none of this matters.  CBOR 
support, on the other hand does matter.

I vote to CONDITIONALLY ACCEPT Boost.JSON, conditional on the inclusion 
of code for parsing and serializing boost::json::value as CBOR.  I can 
live with the added verbosity of Boost.JSON (although I'd rather see it 
reduced where possible), but not without CBOR.

(If CBOR support is not added, this vote should count as an abstain vote 
and not as a reject.  I don't think that Boost.JSON needs CBOR in order 
to be useful to other people - I just don't want to vote to accept a 
library that's not useful for me.)

-- 
Rainer Deyke (rainerd_at_[hidden])