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: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-07-01 20:38:15

On 01/07/2017 20:01, Vinnie Falco via Boost wrote:
> On Sat, Jul 1, 2017 at 11:08 AM, Niall Douglas via Boost
> <boost_at_[hidden]> wrote:
>> Beast WebSocket belongs in another, separate library. That library could
>> bring in ASIO as a dependency.
> 
> Beast is provided as a coherent package of components that work well
> together, with integrated tests and documentation. It is presented as
> a single library.

Except, it is not coherent. You're bundling socket abstraction in with
HTTP parsing and inflicting huge, header-only dependencies on all end
users irrespective of what parts they actually use or need.

At the very minimum, I think Beast needs to become two, separate
libraries: (i) the HTTP utility library (ii) WebSocket.

>> 1. Message containers
>> 2. Stream reading
>> 3. Stream writing
>> 4. Serialisation
>> 5. Parsing
>> You have told me no reason why any of these needs a hard dependency on
>> any networking implementation, or awareness of any specific networking
>> design pattern.
> 
> (repeating myself)
> 
> Beast's message containers, serialization, and parsing do not depend
> on any specific networking interface.

So why are you forcing end users to drag in ASIO?

The reason why according to you is for the buffer infrastructure. But as
I've already told you, that's a relic from a decade ago. New code
neither ought to nor needs to use that. We have far better available today.

And if the really reusable parts of Beast, the ones not dependent on
ASIO except for some buffer adapters, can be broken off and be made free
of ASIO, that's a big value add to end users who don't need WebSocket
and just want a HTTP utilities library.

> I do not know of a way to write an algorithm which works with an
> unspecified stream concept, so I had to choose which concepts I wanted
> to work with. I chose these Boost.Asio concepts because they are the
> closest thing to becoming a standard:
> 
>     SyncReadStream
>     SyncWriteStream
>     AsyncReadStream
>     AsyncWriteStream
>     DynamicBuffer
> 
> Perhaps you can demonstrate how a network algorithm may be written
> which works with an unspecified stream concept? How about a simple,
> synchronous function that writes a string, I'll start you off with a
> function signature:
> 
>     template<class Stream>
>     void write(Stream& stream, std::string_view string);

You're thinking in terms of i/o. Stop doing that. Very little in your
library has, or ought to have, anything to do with i/o. It's the major
flaw in your library's design as I see it.

Think in terms of blobs of bytes. Blobs of bytes come in. Blobs of bytes
go out. No i/o. No reading, no writing. Just blobs of bytes.

>> **HTTP has nothing to do with networking**.
> 
> Beast offers algorithms to serialize and parse HTTP messages on
> Boost.Asio streams. If you think that is not part of "HTTP" that's
> fine, the label is unimportant. What is important is that Beast offers
> this functionality.

i/o, ASIO, networking and all this stuff has confounded the clarity and
intent of your design which is very solid.

Stop thinking in terms of i/o. HTTP is just structured data. So treat it
as such. Parse as structured data, generate as structured data.

The natural split point for Beast into multiple, focused libraries is
between the code which only concerns itself with structured data, and
everything else.

>> WG21 has much superior vocabulary types for doing buffer sequences
>> than ASIO's which are needlessly complex, over engineered, and over wraught
> 
> Please specify the WG21 vocabulary types you are referring to.

The Ranges TS is an obvious place to start from as a source of Concepts
and vocabulary types to draw from. You don't actually need a Ranges TS
as all your Views, InputRanges, OutputRanges etc. will be of char or
const char anyway. These are very easy to knock together, indeed my GSoC
student this year Tom knocked together a constexpr implementation of
those in less than a week. When the Ranges TS is available in your
compiler, of course patch in that if the end user configures a macro for
it, but you can write just enough of a Ranges TS implementation to suit
all your needs very quickly.

If that's too experimental for you (though the Ranges TS is a TS just
like the Networking TS), then do as Boost.Spirit does and work with
iterator pairs. So for example, if I want to know what the
Content-Length header is, upon query I get back an iterator pair
pointing to the front of the storage and one after the end of the
storage. Similarly, if I set the Content-Length header, I supply an
iterator pair to the new contents.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/