|
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