Re: [hash2][review] Early review (due to holidays)

From: Peter Dimov (pdimov_at_[hidden])
Date: 2024-12-04 16:01:18

Claudio DeSouza wrote:
> I tend to disagree. In my opinion even this:
> 
>    hash.update( as_bytes( span{st} ) );
> 
> Is better than this:
> 
>    hash.update( v.data(), v.size() );

Maybe.

But the two cases with the current interface are consistent - you do the same
thing for both vector and string - and the two cases with the "normal" span
aren't.

I would prefer a span-style interface where the two cases look the same,
like this

    hash.update( v );
    hash.update( st );

There'd be no questions here as to whether this is a usability gain compared
to the current interface.

(Ideally, in a hypothetical stdlib, .data would return a span and the above
would be

    hash.update( v.data() );
    hash.update( st.data() );

to make it explicit that we're dropping to a lower level interface, but one
can't have everything.)

> Something like as_byte_span is just an extra nicety, but I think it is easy to
> underestimate how you can avoid mistakes with the first example as opposed
> to the second.
> 
> On Wed, Dec 4, 2024 at 2:38 PM Peter Dimov via Boost
> <boost_at_[hidden]>
> wrote:
> 
> > Claudio DeSouza wrote:
> > > > So I switched `update` back to void const*. The "type safety"
> > > > gains aren't worth the substantial decrease in usability.
> > > >
> > >
> > > In my experience, the usability/readability gains are on the side of
> > using span.
> > > This may involve providing extensions, but at the end of the day you
> > > get
> > more
> > > C++-friendly constructs.
> >
> > Well... no, they aren't.
> >
> > Suppose you have the data in `std::vector<unsigned char> v`. With the
> > current `update` that's
> >
> >     hash.update( v.data(), v.size() );
> >
> > And if you have the data in `std::string st`, that's
> >
> >     hash.update( st.data(), st.size() );
> >
> > Now suppose `update` takes `span<unsigned char const>`. For `v`, this
> > is
> >
> >     hash.update( v );
> >
> > which is nice; but for `st`, that would be (assuming C++17)
> >
> >     hash.update( as_bytes( span{st} ) );
> >
> > which is not so nice.
> >
> > In Chromium you have as_byte_span, which does as_bytes(span(x)), so
> > it'd be better.
> >
> >
> >
> > _______________________________________________
> > Unsubscribe & other changes:
> > http://lists.boost.org/mailman/listinfo.cgi/boost
> >
> 
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost