Re: [review][json] My review

From: Julien Blanc (julien.blanc_at_[hidden])
Date: 2020-09-23 06:31:30

Le mercredi 23 septembre 2020 Ã  14:08 +1200, Gavin Lambert via Boost a
écrit :
> Mere moments ago, quoth I:
> > On 23/09/2020 12:52, Vinnie Falco wrote:
> > > It is the opposite of SRP violation. basic_parser is concerned
> > > only
> > > with parsing and not keeping track of the full value. The
> > > responsibility of tracking full values is moved to the caller.
> > > That
> > > way, basic_parser does not impose its opinions on how the full
> > > value
> > > should be stored. This was litigated in earlier posts if I recall
> > > (Peter?).
> > 
> > Except it does, because it parses into int/uint/double, which is 
> > inherently a storage decision.  You might have a valid argument if
> > it 
> > left that step to the handler to determine as well.  (i.e. only 
> > providing on_number_part and on_number, just the text without any 
> > attempt to parse into a numeric type.)
> 
> It's not just a storage *decision*; in order to implement this
> interface 
> with the observed behaviour it also means that basic_parser must 
> currently actually have *storage* for partial numbers updated across 
> multiple fragments.  This is inconsistent with how it treats keys
> and strings.

The base inconsistency is the fact that storing a number, whatever
large it is, takes a fixed amount of storage (talking about doubles or
longs, which are the only types handled by boost.json), whereas for a
string it is not the case. A non-allocating parser can safely handle
integers and double itself (and it can do it in a more efficient way
than if it's done in an upper layer), but has no other choice than
splitting strings (unless there is an upper limit on string size).

> I still think that it would be better to instead provide an
> interface 
> that precomposes the text fragments, but that is a separate argument 
> from the prior one, that can be treated separately.
> 
> Perhaps you could provide both -- basic_parser as-is (but with the 
> number parsing/storage removed), and another layer on top 
> (custom_parser, perhaps) that is nearly identical but omits the
> _part 
> methods to only provide precomposed keys/values.  This is the one
> that 
> most consumers of a SAX-style interface would be expected to actually
> use.

>From what i've seen, such an interface would be quite easy to
implement. That would break the purpose of the non-allocating parser,
though, so i'm not sure it would be that useful.

Regards,
Julien