Boost logo

Boost :

From: Peter Koch Larsen (peter.koch.larsen_at_[hidden])
Date: 2019-12-04 20:29:44


Dear boosters

Here is my review of fixed_string
> - What is your evaluation of the design?

I have just completed a similar but much less ambitious design - about
700 LOC in total. The purpose is to replace char string[N]
representing string in an ageing code base that is C++98 currently and
partly embedded. My design does support a partial compatibility with
std::string but that was not the primary goal.

The interface compatibility of std::string is a nice feature, but I do
not see a reason to attempt to make it a drop-in replacement.
The non-compatibility with char[N] is missed by me and probably also
some other in the embedded domain. It would be nice if fixed_string<N>
had size = N + 1 for char types.

> - What is your evaluation of the implementation?

Nice and clean. As others have noted, constexpr and noexcept support
would be nice. I would also prefer if I could have an option that does
not require usage of exceptions, perhaps as a configuration option.

Specifically, I would like to be able to control error handling
(terminate instead of throw, ability to react to undefined behaviour).

Also, I would like to have more forgiving functions that could return
the status of failing operations.

> - What is your evaluation of the documentation?

Nice and clean.

> - What is your evaluation of the potential usefulness of the library?

If the interface could be extended, It could potentially be useful to
many - and to me - if I manage to convince my company to ugrade to
C++17. So to summarize:

1) compatibility with char[N]: this requires that the size member is
removed. In my source, I still have an O(1) size implemented by having
data[N] give the free space in the string.

If data[N] == 255, this indicates that the real free size is stored in
the elements data[N - 1] and below. There are many ways to encode that
length portably.

2) append and assign could have companion functions, that behave in
spirit like strncpy/strncat. I have named these function try_append
and try_assign.

3) Errorhandling strategy available.

> - Did you try to use the library? With what compiler? Did you have
> any problems?

No, just reading code.

> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

About three hours reading and writing this comment.

> - Are you knowledgeable about the problem domain?

Yes.

>
> And finally, every review should answer this question:
> - Do you think the library should be accepted as a Boost library?

The constexpr and noexcept parts should be fixed and compatibility
with char[] would be nice. But yes, it should be accepted.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk