Re: [boost] [review][beast] Review of Beast starts today : July 1 - July 10

Subject: Re: [boost] [review][beast] Review of Beast starts today : July 1 - July 10
From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2017-07-02 01:20:08

On Sat, Jul 1, 2017 at 7:22 PM, Vinnie Falco via Boost <
boost_at_[hidden]> wrote:

> On Sat, Jul 1, 2017 at 4:42 PM, Zach Laine via Boost
> <boost_at_[hidden]> wrote:
>
> Thanks for checking out the library!
>
> > * Is there a reason why buffer_cat_view and buffer_prefix_view are
> distinct
> > types?
>
> Yes, buffer_cat_view and buffer_prefix_view are two completely
> different things. A buffer_cat_view represents a buffer sequence of
> buffer sequences, while a buffer_prefix_view is just a range adapter
> over a single buffer sequence.
>
> > I think you just need to make a single type that owns a sequence of
> buffers,
> > and maintains a pair of iterators or indices into the owned sequence.
>
> You've more or less described buffer_prefix_view.
>

Right.

> A rough description of buffer_cat_view would be "a type that owns a
> heterogenous list of buffer sequences, whose iterators reflect the
> current position within a variant consisting of each iterator types
> from the list of buffer sequences." Quite different :)

They don't seem that different to me.  The data look the same from the
outside, but the underlying structure is different.  The underlying
structure doesn't seem like it *needs* to be different, though.  One could
be flattened into the other.  These are all views, right?  Cheap-to-copy
views?  So, in terms of the existing types, if buffer_cat() returned a
buffer_prefix_view with n=0, would that break things?  I mean a semantic
breakage, not a compilation breakage.

> > * Similarly, as a user I'd prefer not to have string_body as a distinct
> > type (unless it's a convenience typedef for dynamic_body<std::string>).
> If
> > std::string is not a model of DynamicBuffer, I think DynamicBuffer could
> > stand a slight reformulation.
>
> std::string does not model DynamicBuffer, which itself is a Net-TS
> concept and thus immutable from Beast's perspective. Therefore
> beast::http::basic_dynamic_body cannot work with std::string and
> beast::http::string_body becomes necessary . I will likely add
> beast::http::basic_string_body to allow the allocator to be
> customized.

I get it.  That's a shame.

> > * Allocators are gross.  I know this is a controversial position,
>
> I agree, that is controversial! And I'm not entirely happy with the
> interface of AllocatorAwareContainer but it is what it is. Which is to
> say it is standardized.

It's not that uncommon an opinion these days.  Be mentally prepared for a
hard left turn wrt allocators (which may never come), if this finally makes
it to LEWG.

> > * Please add an explicit Rationale section for smaller design choices.
> The
> > FAQ and design justification sections that exist are fine, but they
> mostly
> > cover the biggest-picture items. Each of those small design choices are
> > going to go out the window if you can't remember a strong argument for
> one
> > of them 5 years from now, when LEWG are asking you why a certain
> interface
> > is the way it is.
>
> Good advice but either vague or broad in scope (every design choice?),
> if there are specific design choices that should be explained then
> this becomes more actionable:
> <https://github.com/vinniefalco/Beast/issues/569>
>

Well, certainly not every choice.  But definitely every nonobvious choice,
or a choice that looks from the outside like taste, should go in a
rationale section. imo.

Zach