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

On Sat, Jul 1, 2017 at 7:22 PM, Vinnie Falco via Boost < boost@lists.boost.org> wrote:

On Sat, Jul 1, 2017 at 4:42 PM, Zach Laine via Boost 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_bodystd::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