Re: [Boost-users] [review][fixed_string] FixedString review starts Nov 25

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Boost-users] [review][fixed_string] FixedString review starts Nov 25

Boost - Dev mailing list
This is my review of the proposed fixed_string library.

Notes:

   - The list of member functions should include a full declaration so I
   know what's what. Other than that it does the job, though it would be nice
   to have a hierarchical TOC on the side for reference.

   - Add constexpr and noexcept where appropriate.

   - There was a discussion on whether or not resize should call
   boost::throw_exception. It should (as it does) because A) buffer overruns
   are gnarly and B) the overhead of the check is negligible, considering this
   is supposed to be a universal library.

   - Operator+ should be provided. I'm not sure whether it should use
   capacity N+M or something else, but all technical questions
   notwithstanding, users ought to be able to concatenate strings easily, e.g.
   if I want to add a "ms" to a number I shouldn't be jumping through hoops.

   - It's nice that the authors are concerned about physical coupling, but
   there are good reasons to provide (in addition) a single-header version
   (e.g. Godbolt).

My vote is to accept.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost-users] [review][fixed_string] FixedString review starts Nov 25

Boost - Dev mailing list
On 5/12/2019 11:32, Emil Dotchevskit wrote:
>     - Operator+ should be provided. I'm not sure whether it should use
>     capacity N+M or something else, but all technical questions
>     notwithstanding, users ought to be able to concatenate strings easily, e.g.
>     if I want to add a "ms" to a number I shouldn't be jumping through hoops.

Is there some reason why you consider operator+= insufficient for that
purpose?


Perhaps this needs to be stated more clearly in the docs, but the
impression I get is that the primary intended usage is something like:

     fixed_string<512> buffer;    // some arbitrarily large N
     buffer += prefix;
     buffer += suffix;
     consume(buffer);

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost-users] [review][fixed_string] FixedString review starts Nov 25

Boost - Dev mailing list
On Wed, Dec 4, 2019 at 2:45 PM Gavin Lambert via Boost <
[hidden email]> wrote:

> On 5/12/2019 11:32, Emil Dotchevskit wrote:
> >     - Operator+ should be provided. I'm not sure whether it should use
> >     capacity N+M or something else, but all technical questions
> >     notwithstanding, users ought to be able to concatenate strings
> easily, e.g.
> >     if I want to add a "ms" to a number I shouldn't be jumping through
> hoops.
>
> Is there some reason why you consider operator+= insufficient for that
> purpose?
>

There are two use cases:

   1. The user knows he is working with fixed_string, and he chooses to
   call op+. There's no reason to take away this choice.

   2. The user has a generic function that concatenates "strings". We need
   good reasons if we want to force him to rewrite the function. It is
   possible that there could be some problem with op+, but at this point it
   seems mostly theoretical.

I presume the second case is rather rare.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost-users] [review][fixed_string] FixedString review starts Nov 25

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
śr., 4 gru 2019 o 23:33 Emil Dotchevski via Boost <[hidden email]>
napisał(a):

> This is my review of the proposed fixed_string library.
>
>
>    - There was a discussion on whether or not resize should call
>    boost::throw_exception. It should (as it does) because A) buffer
> overruns
>    are gnarly and B) the overhead of the check is negligible, considering
> this
>    is supposed to be a universal library.
>
>    - Operator+ should be provided. I'm not sure whether it should use
>    capacity N+M or something else, but all technical questions
>    notwithstanding, users ought to be able to concatenate strings easily,
> e.g.
>    if I want to add a "ms" to a number I shouldn't be jumping through
> hoops.
>

One possibility here, assuming that
* concatenating fixed_string<N> with std::string is out of the question and
*  fixed_string<N> is a drop-in replacement for std::string and
* resizing over capacity is treated as a correct (albeit rare) situation in
the program

is to simply provide `fixed_string<N> operator+(fixed_string<N>,
fixed_string<N>)`.
This will be useful when some part of the program uses all `fixed_string`s
with the same capacity anyway. If fixed_string is a drop-in replacement for
string one would not be surprised with this signature.
For std::string, one is not surprised if string a was able to store its
contents, and b was able to store its contents, but there was no room to
store the concatenated version of a and b and this resulted in an exception.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost