Re: [JSON][review] a small review of boost.json

From: Vinnie Falco (vinnie.falco_at_[hidden])
Date: 2020-09-19 12:37:35

On Sat, Sep 19, 2020 at 3:32 AM Julien Blanc via Boost
<boost_at_[hidden]> wrote:
> However, it
> resides in a <detail> header, which would indicate that it is not an
> expected use case. I believe it should be.
>
> The callbacks of the parser (push parser) are not documented. I'm a bit
> puzzled by what the on_number_part does, and how it fits with the
> on_int64/on_double callbacks, for example.

basic_parser is a class template whose header is in
<boost/json/detail/basic_parser.hpp>

The member function template definitions for this class are in
<boost/json/basic_parser.hpp>

When you include <boost/json.hpp> (or any other public header, except
for basic_parser.hpp) you do not get the class declaration or the
definition for basic_parser. This is by design. A goal of the library
is to be quick to compile. Thus the source code is structured to
expose as few function definitions as possible (but still provide
important functions as inline functions, to aid the optimizer).

It *is* possible to use basic_parser with your own handler (and it is
documented). You just need to include the header manually, like this:

    #include <boost/json/basic_parser.hpp>

This gives access to the class declaration and the member function
definitions. The callbacks are documented here:

<https://master.json.cpp.al/json/ref/boost__json__basic_parser.html#json.ref.boost__json__basic_parser.handler0>

> I could not find a way to make :
>
>     auto v = p.write(R"json({"foo":1}{"bar":2})json");

`write()` returns an error if it does not consume all the input. You
need to call `write_some()`, which will work exactly the way you want.
This is even documented :) The second to last code snippet in this
subsection:

<https://master.json.cpp.al/json/usage/parsing.html#json.usage.parsing.parser_instance>

> The json specification, AFAIR, does not forbid to rely on a specific
> order of the keys. Some serialization libraries do expect the keys to
> be in a specific order.

Boost.JSON keeps the order after parsing, if there are no duplicates,
but does not guarantee preservation of ordering after the value is
modified. The JSON spec says that the objects are unordered.

> The to_double
> function should silently convert integers to their double equivalent
> whenever it is possible.

There's no `to_double`, maybe you mean `as_double`? In any event, the
function that does what you want is called json::number_cast

> It produces a strange representation for numbers, but it is conforming
> to the JSON specification.

Yes the subject of converting floating point to string is an area of
improvement.

> Another thing that the doc should state clearly : boost.json is **not**
> a JSON (ECMA-404) library, but rather an I-JSON (RFC 7493) library with
> some extensions.

> The second one was incompatibilites between standalone asio bringing
> boost headers (because boost is installed on my machine) and json
> redefining them (because of standalone mode), resulting in multiple
> defenitions of throw_exception.

I'm open to suggestions on how to fix it:

<https://github.com/CPPAlliance/json/blob/8c1075ab0bdcd0691ebc6f3574b9988cc5d3b2f6/include/boost/json/detail/impl/except.ipp#L37>

> it does not address every use
> case in the world, but it does a very good job at what it is designed
> to do. It is easy to use, performant, and it addresses a very
> widespread use case

I fully agree with this description, that is precisely what was intended.

> I think the library should be ACCEPTED.

Thank you for your thoughtful review and time, and for submitting a
pull request!

Regards