Design review
I mentioned in a comment that you could use a std::string
for this. However, that would be a quick-and-dirty solution. When you have a requirement that has to be true, making a custom type is exactly the right solution. Once you have a working type that enforces the constraints you need, everything else is trivial, and safe by default.
What is the point?
You have neglected to explain why you need this particular abstraction, instead offering only vagaries like “a very specific use case”. Because of that, you can’t possibly get a good code review. Without knowing what the problem is, no-one can possibly tell you whether your solution is good, or completely idiotic.
Even just trying to consider the idea in the abstract is difficult, because to know whether an interface is “good” or “useful”, you have to know what a “normal” usage would look like. Like, does it make sense to silently “hide” the prefix and postfix NUL characters in functions like .at()
or .empty()
… but not hide them in .print()
?
Here’s the bottom line: You have claimed you “need” a string with NULs at either end… but you have made a string that hides the NULs at either end. So… huh? 🤷🏼 Do you need the NULs or not? And if you really do need the NULs, why does your interface work so hard to make them disappear?
Another thing that makes me scratch my head is the distinction between “text” and “bytes”. It seems like you intend for the type to “look like” a string through the “text” view, but provide access to the NUL-capped data through the “bytes” view, which is fine… except then you have a bunch of functions that make the “text” view the “true” view (like .size()
). So these are not two different ways to view the same data, but rather one true/correct view (as “text”), and one sort of second-class citizen view (that is, ironically, the actual truth of the data).
So when I ask “what is the point?”, I am not implying the type is useless, but rather that you are confused about what it will be used for. Is it a string? Or is it a container of character data that is guaranteed to be NUL terminated at both ends?
The name
@J_H already mentioned the problem with “zero”. A “left zero-padded string” already means something: "00000foo"
. (That would be “foo” left zero-padded to a width of eight.) The correct name for the character is “NUL”. Some people also use “NULL”, but be careful, because that can also mean the null pointer.
So what you want is not LeftZeroPaddedString
, it is LeftNulPaddedString
.
Except… that’s still not correct.
Because “padding” is not what you are doing. When I see the name LeftZeroPaddedString
(or LeftNulPaddedString
), I expect that the type allows me to specify a string and a width, and if the string length is less than the width, it fills up the “left” side with extra zeros (or NULs) to make up the space.
I’ll also point out that the “left” part is dodgy, too. Historically, people just assumed all text was English… but this is the 21st century. The beginning of a string is not necessarily the left part of the string. That depends on what the language is. Some languages are right-to-left. Others are top-to-bottom! (There are even a very small number of bottom-to-top languages.)
So I would suggest what you are making is not a “left zero-padded string”, but rather a “NUL-prefixed string”. “Prefix” is valid for any language direction.
Or maybe it’s not a string at all. Maybe it’s a “NUL-bordered character array”.
Figuring out exactly what it is is an important first step is getting the abstraction right.
vector
is a poor choice
You have based your type on std::vector<char>
. I assume that is because you were not aware you could put NUL anywhere in a std::string
. But you can!
And because you can use std::string
, you should use it. It is better than std::vector
, if only for the fact that the small-string optimization is possible. Because of the small-string optimization, you can assume that, generally, any string of less than 15 characters will not require any dynamic memory allocation; it can all be stored very efficiently within the string itself. And short strings are very common. In fact, your example string—"Test wefjwef"
—even with the extra NUL, would fit.
However, because you have used std::vector
, even an “empty” string requires dynamic allocation. That is expensive, and dangerous, because users often count on noexcept
default construction for strings… but you can’t offer that.
If std::vector
supported a “small-string optimization”, then I would enthusiastically recommend it over std::string
, because I think this type should be an abstraction of “container of bytes”, not “sorta-kinda string”.
Inheritance is a poor choice
There is really no sensible reason to inherit from std::vector
(or std::string
), rather than to include one as a data member. In practice, you have to override or re-implement virtually every function anyway, so there isn’t even any code reuse benefit.
There is a general rule: prefer composition to inheritance. Technically this doesn’t really apply here, because you are using private inheritance. But still, using data members is just the more “natural” and “clear” way to to write classes.
We’ll see more reasons why inheritance is bad design later on.
The interface
You are making a string, basically. Thus, it should have a string-like interface.
Now, there are actually two options for this. One option is just to look at the interface for std::string
, and duplicate it. The other option is… not: Make whatever interface you like for your type, but provide a conversion to std::string_view
.
I think the best option in your case is to not duplicate the std::string
interface, because your class is… weird in a lot of ways.
Instead, I think you should just create a container-like interface that does not hide the leading and trailing NULs… and provide conversions to std::string
and std::string_view
.
Put another way: When I asked whether the type is supposed to be a string or a character container that is NUL terminated at both ends… I am suggesting the answer should be the latter. Don’t pretend this is a simple string. Don’t fake being a string by lying in functions like .size()
and .empty()
. Accept what this is: a special container for string data that NUL terminates both ends. It is not a string itself, but it can provide string views of the data between the NULs.
(Then, the next thing to consider is whether it should be valid for an “empty” object to have only a single NUL, or if it must have two. Again, no-one can guess what the correct answer to this is without knowing more about your use case.)
I could tell you that your abstraction is a mess, but I think the most effective way to prove the point is with an illustration. What should be the output of the following program?
auto main() -> int
{
using namespace std::literals;
auto const v = LeftZeroPaddedString{"foobar"sv};
std::println("size = {}", v.size());
std::println("distance(begin, end) = {}", std::ranges::distance(v.begin(), v.end()));
std::println("distance(bytesBegin, bytesEnd) = {}", std::ranges::distance(v.bytesBegin(), v.bytesEnd()));
std::println("distance(textBegin, textEnd) = {}", std::ranges::distance(v.textBegin(), v.textEnd()));
}
Here’s what I got:
size = 6
distance(begin, end) = 6
distance(bytesBegin, bytesEnd) = 7
distance(textBegin, textEnd) = 5
Now, I’ll accept that the “text” distance should be two less than the “bytes” distance, because the “bytes” distance includes the two NULs. But how is it that neither the “text” distance nor the “bytes” distance is the same as the size?!
Clearly something has gone wrong.
char
/byte confusion
A char
is not a byte. They are defined to be the same size, yes, but they are distinct types, and totally different abstractions that represent different concepts.
See for yourself.
Yes, a char
can be converted to a byte, and vice versa, without loss of information. But they are not the same.
As I said, I think you should make this abstraction a container of bytes, and provide string views as an extra interface. So this shouldn’t be based on std::vector<char>
, it should be based on std::vector<std::byte>
.
What about allocators?
So, I assume you have no interest in supporting other character types (because that would completely break the whole text/bytes dichotomy). I also assume you have no interest in character traits (because, again, I don’t think you really mean for this type to be a string abstraction, I think you mean for it to be a byte container, that has some string view capability).
However, why not support allocators?
After all, you get allocator support more or less for free, whether you use std::vector
or std::string
as the underlying type.
You could transform the type to a template, and add an allocator template parameter. Or you could avoid having to make the class a template by using polymorphic allocators.
Either way, you should consider allocator support.
You need to test your code
One of the most important skills you can learn as a programmer—not just for C++ but for any language—is testing.
I can tell you did not test your code. I can tell because—not only does it have some nasty and rather obvious bugs that would have been trivially found with even a basic testing setup—I can tell the code was not tested because its design is broken.
Here’s the thing with testing: it doesn’t just help you find bugs and broken code. If you write your tests first, it forces you to think about the design, and make it useful and sensible from the start.
I pointed out above that this type presents at least three different sizes: the .size()
size, the “text” range size, and the “bytes” range size. That’s nonsense, and there is no way anyone would have made a class that does that by design. The only way this could happen is by just blindly slapping together a class, without proper forethought and planning.
TEST YOUR CODE. And by that, I don’t mean “slap together a simple program with some asserts, or just print some stuff then read it to see if it’s right”. I mean use a proper testing framework, and—and this is very important—write the tests first, before you write the class. Think about your expectations for how things should work, write the tests to check for those expectations, then write the code to pass the tests. That is the path to good code.
Code review
explicit constexpr LeftZeroPaddedString(const char* _begin, const char* _end)
: std::vector<char>((_end - _begin) + 2)
{
this->front() = '\0';
if(_begin != _end)
{
std::copy(_begin, _end, std::vector<char>::begin() + 1);
}
this->back() = '\0';
}
explicit constexpr LeftZeroPaddedString(const std::string_view &_text)
: LeftZeroPaddedString(_text.begin(), _text.end())
{ }
explicit constexpr LeftZeroPaddedString(void)
: LeftZeroPaddedString("")
{ }
So these are your three constructors. You have chosen to use the first one—the one that takes an iterator pair (technically, a pair of pointers)—as the “base” constructor, and make every other constructor delegate to that.
Here’s the problem with that: that iterator pair constructor is a rather “expensive”, and “risky” constructor, because it needs to allocate memory, and because it might fail. Also, if it were written well, it would have to do quite a significant number of checks. For example, it needs to check that both pointers are not nullptr
, and that they are a valid range. If any of those checks fail, again, the constructor might fail.
All of this is signalling that this is not the right way to do this.
Let’s go in reverse order and start with the default constructor. Now, the default constructor is a critical part of a class. It’s quite peculiar to have it delegate to anything else. But even if it does, it should still be noexcept
, if at all possible. With vector
as the base, that is not possible… but it is possible with string
as the base (so long as you allow a single NUL in the empty case… if not, then you would have to implement everything manually, which, honestly, would not be all that hard).
But the important thing is: implementing the default constructor in terms of other public constructors is a bad idea, because other public constructors will have to do checks on the input data that will just be a waste of time for the default constructor (because you control the input data in that case).
You can implement the default constructor in terms of other constructors, but they should either have a wide contract or be private. They must be constructors that have no need to do any checks on the arguments, either because all arguments are valid (so checking is unnecessary) or you are sure it can never be called with invalid arguments (because it is private).
So, for example, with a private constructor as your basis constructor:
struct internal_t {};
class LeftZeroPaddedString
{
private:
constexpr explicit LeftZeroPaddedString(internal_t, std::string_view sv)
{
_data.resize(2 + sv.size());
std::ranges::copy(sv, _data.data() + 1);
_data.front() = '\0';
_data.back() = '\0';
}
static constexpr auto _validate(char const* first, char const* last)
{
if (first == nullptr)
throw std::invalid_argument{"first is nullptr"};
if (last == nullptr)
throw std::invalid_argument{"last is nullptr"};
// other checks...
return std::string_view{first, last};
}
static constexpr auto _validate(std::string_view sv)
{
// any checks you want...
return sv;
}
std::vector<char> _data;
public:
constexpr explicit LeftZeroPaddedString()
: LeftZeroPaddedString{internal, std::string_view{}}
{}
constexpr explicit LeftZeroPaddedString(char const* first, char const* last)
: LeftZeroPaddedString{internal, _validate(first, last)}
{}
constexpr explicit LeftZeroPaddedString(std::string_view text)
: LeftZeroPaddedString{internal, _validate(text)}
{}
But let’s get back to the code as actually written:
explicit constexpr LeftZeroPaddedString(void)
: LeftZeroPaddedString("")
{ }
First, we don’t put void
in empty argument lists. That is an archaic C practice.
Second, there doesn’t really seem to be a reason to make this constructor explicit
. That just makes using this type more verbose, for no good reason.
Third, if you are making an empty std::string_view
, using ""
is a bad idea. When you do ""
, you are calling the string view constructor that takes a pointer to a NUL-terminated character string. That constructor has to be quite a bit of work. It has to do a bunch of checks (is the pointer nullptr
?), and then it has to de-reference the pointer, and do a string length check. With luck, the compiler can optimize all this away… but why not just say what you mean instead? You don’t want a string view of ""
. You want an empty string view. Don’t get there the hard way. Be explicit and direct: std::string_view{}
.
So:
constexpr LeftZeroPaddedString()
: LeftZeroPaddedString{std::string_view{}}
{}
Now, as for the other two constructors, it is technically irrelevant whether you implement one in terms of the other, or vice versa. However, I would always recommend using the “safer” alternative as the basis operation. If the “safer” alternative is the basis, then when you use it directly, you don’t need to do the extra checks required for the “less-safe” alternative. But if the “less-safe” alternative is the basis, you always pay for those checks, even when unneeded.
In this case (and in general), the range version is safer, because there is no chance of the iterators being from different ranges, or mixing up the begin/end iterator, etc.. So the basis should be string view… not the iterator pair.
explicit constexpr LeftZeroPaddedString(const std::string_view &_text)
: LeftZeroPaddedString(_text.begin(), _text.end())
{ }
First you should never take view types by reference. They are meant to be taken by value.
Second, in C++, we keep type information together. That means:
const std::string_view &_text
: This is C-style.
const std::string_view& _text
: This is C++-style.
std::string_view const& _text
: Also C++-style.
Third, if you are going to use an underscore prefix, the place to use it would be for private data members… not function arguments. Function arguments take priority, so there is no sensible reason to mangle them.
Fourth, and this is the most important thing (and another reason why you should make the string view constructor the basis constructor)… std::string_view::begin()
does not return a char
pointer (const
or otherwise). It returns a std::string_view::iterator
. Now, std::string_view::iterator
could be a char
pointer (and I think it is in both libstdc++ and libc++)… but it does not have to be.
So what you would have to do is something more like (_text.data(), _text.data() + _text.size())
… except that’s dodgy, because _text.data()
might be nullptr
, which you shouldn’t be adding anything to (including zero).
There are two options here:
- Do it the other way around. Implement the
char
pointer version in terms of the string view version.
- Change the
char
pointer version to a general iterator version.
Option 1 I’ve illustrated above.
Option 2 would look something like:
template <std::input_iterator I, std::sentinel_for<I> S>
requires std::indirectly_copyable<I, char*>
explicit constexpr LeftZeroPaddedString(I first, S last)
{
if constexpr (std::forward_iterator<I>)
{
_data.resize(std::ranges::distance(first, last));
auto p = _data.data();
*p++ = '\0';
*(std::ranges::copy(sv, p).out) = '\0';
}
else
{
_data.push_back('\0');
std::ranges::copy(first, last, std::back_inserter(_data);
_data.push_back('\0');
}
}
explicit constexpr LeftZeroPaddedString(std::string_view text)
: LeftZeroPaddedString(text.begin(), text.end())
{}
And the last constructor:
explicit constexpr LeftZeroPaddedString(const char* _begin, const char* _end)
: std::vector<char>((_end - _begin) + 2)
{
this->front() = '\0';
if(_begin != _end)
{
std::copy(_begin, _end, std::vector<char>::begin() + 1);
}
this->back() = '\0';
}
This is where your idea of inheriting from vector
really begins to stink (though it gets much, much worse later).
Firstly, I should note that we don’t write this->
in front of every member function or data member. It is very noisy.
But the real ugliness is here: std::vector<char>::begin()
.
Now, you know why you have to write this. It is because you have overridden begin()
with a whole new implementation… one that skips that initial NUL. In order to get back the original vector
implementation, you need that ugly and verbose incantation.
But here’s a damning question: What happens if, at some point in the distant future, you or someone else decides to also override front()
, and have it return a reference to the first character after the initial NUL?
Oops.
Now, you can “guard” against this by uglifying every single vector
call the same way you did for begin()
here, and get this:
constexpr explicit LeftZeroPaddedString(char const* first, char const* last)
: std::vector<char>(std::ranges::distance(first, last) + 2)
{
std::vector<char>::front() = '\0';
std::copy(first, last, std::vector<char>::begin() + 1);
std::vector<char>::back() = '\0';
}
And of course, you can “guard” every single case where you call a vector
function and want the vector
implementation.
So… problem solved?
No.
Look at this function:
constexpr bool empty(void) const
{ return this->byteSize() == 2; }
“What’s the problem?”, you ask. “std::vector
doesn’t have a .byteSize()
function!”
You are correct…
…
… at least, up to C++23. And C++26 so far.
But std::vector
might get a .byteSize()
function in the future… and if/when that happens… boom.
And this is why implementing via inheritance is such a terrible idea. When you just include a vector
in your type, you are insulated from any changes to vector
. So long as the interface you use remains consistent, the standard can do whatever it pleases to vector
; it can add new public
or protected
data members or member functions (or even static stuff), and you won’t be affected. But when you inherit from vector
… you are a vector (even if privately), so any changes to vector
change you. So ever time the standard gets updated, you will need to check that it hasn’t broken your class.
So, in summary:
- Don’t implement your default constructor in terms of other constructors, unless they are no-fail or private.
- Don’t implement range constructors in terms of iterator constructors. Ranges are always better than iterator pairs.
std::string_view::iterator
is not necessarily a char const*
.
- Don’t inherit from stuff you don’t have to. Prefer composition to inheritance.
Moving on.
explicit constexpr LeftZeroPaddedString(const LeftZeroPaddedString& _other)
{ *this = _other; }
Don’t write special member functions if you don’t have to.
You could just do this:
// Note the `explicit` is utterly pointless here.
constexpr LeftZeroPaddedString(LeftZeroPaddedString const&) = default;
But even that is not as good as:
// ಠ_ಠ
Yes, that’s right: nothing.
The implicitly-defined default operations are just fine. So leave them implicitly defined.
In fact, by defining this constructor (and the move ops later), you have disabled move construction and move assignment. You have crippled the efficiency of the type… for nothing.
So just… don’t do it.
(Also, as an aside: You are implementing the copy constructor of a derived type, but for some reason, you don’t call the copy constructor of the base type (and, instead, you call the default constructor, then do copy assignment). That’s unwise.)
constexpr size_t byteSize(void) const
{ return std::vector<char>::size(); }
constexpr size_t size(void) const
{ return this->byteSize() - 2; }
constexpr bool empty(void) const
{ return this->byteSize() == 2; }
So you have a “size” which is not actually the size of the data, and a function called “empty” that returns true if the size is 2.
Clearly something is wrong with this abstraction.
As I’ve suggested, don’t twist yourself in knots trying to create a string view interface that lies. Create an honest interface, and provide a string view conversion that gives you a view of the “text” data.
So, drop .byteSize()
, and all of the other .bytes*()
functions (like .bytesBegin()
. Just let .size()
return the truth: how many characters are in the container… including the NULs. And let all the other container functions also return truth: .begin()
points to the first NUL, and .end()
points to one past the last NUL.
Some other comments:
- There is no
size_t
. It is std::size_t
, and you need to include the correct header. (It “works” for historical reasons, but that will change in the future.)
- These functions cannot possibly fail, under any circumstances. So they should be
noexcept
.
So:
constexpr auto size() const noexcept { return _data.size(); }
constexpr auto empty() const noexcept { return _data.empty(); }
Much simpler, eh?
constexpr char *bytesBegin(void)
{
assert(std::vector<char>::operator[](0) == '\0');
return &std::vector<char>::operator[](0);
}
constexpr const char *bytesBegin(void) const
{ return (const_cast<LeftZeroPaddedString*>(this))->bytesBegin(); }
Implementing the otherwise-identical const
and non-const
versions of a function using const_cast
is an acceptable hack, but it would be wiser to implement the non-const
version in terms of the const
one.
The way you are doing it now, you are removing a const
from something that comes with it. Is that safe?
On the other hand, if you did it the other way around:
constexpr auto bytesBegin()
{
return const_cast<char*>(std::as_const(*this).bytesBegin());
}
constexpr auto bytesBegin() const
{
// implementation
}
Now you are adding the const
to something… and then removing it from the result. And you know this is safe because you know this
was not const
to begin with.
That is the original way Meyers (I believe) formulated it.
Of course, there is a better solution these days:
template <typename Self>
constexpr auto bytesBegin(this Self&& self) const
{
return std::forward<Self>(self).data();
}
But that’s another story.
constexpr char *bytesEnd(void)
{
assert(std::vector<char>::operator[](std::vector<char>::size() - 1) == '\0');
return &std::vector<char>::operator[](std::vector<char>::size() - 1);
}
What… what?!
Are you trying to confuse people and create bugs.
The universal, standardized definition of “begin” and “end” in C++ is a half-open range, where “begin” points to the first item in the range, and “end” points to one step past the last item.
You have defined .bytesBegin()
to point to the first item (the initial NUL). That’s fine.
But then you define .bytesEnd()
to point to the last item (the final NUL)… not the one past the last item!!!
That’s absurd. Did you try to use the type? Did you even test it? Everything will break. Nothing will work as you expect.
In fact, you even wrote your string view conversion like this:
std::string_view(this->textBegin(), this->bytesEnd());
The hell? Did you not see using a different types of iterators in a single operation like this as a problem? If you’d actually implemented real iterator types, rather than just using pointers, this would not even have compiled. (This is why it is a good idea to use real iterator types, at least when debugging, even if raw pointers would work.)
We’ll be coming back to this mess later.
constexpr char *textBegin(void)
{
if(this->empty())
{ return this->bytesBegin(); }
else
{
return this->bytesBegin() + 1;
}
}
constexpr const char *textBegin(void) const
{ return (const_cast<LeftZeroPaddedString*>(this))->textBegin(); }
constexpr char *textEnd(void)
{
if(this->empty())
{ return this->bytesBegin(); }
else
{
return this->bytesEnd() - 1;
}
}
constexpr const char *textEnd(void) const
{ return (const_cast<LeftZeroPaddedString*>(this))->textEnd(); }
I think you’ve kinda lost the plot here (which is another sign that your abstraction is a mess).
When .empty()
is true, there are 2 NULs in the data. Always always. .empty()
is literally defined as “size equals two”. .bytesBegin()
always points to the initial NUL. .bytesEnd()
always points to the trailing NUL. You have asserted these facts (misguided though they may be, in the latter case).
So… what is the point of doing the checks and branching. By definition…:
constexpr auto textBegin() { return ++bytesBegin(); }
constexpr auto textEnd() { return --bytesEnd(); }
… must be correct.
And they cannot even conceivably fail, so you could even make them noexcept
.
void print(void) const
{
for(const char& _c: *static_cast<const std::vector<char>*>(this))
{
std::cout << "" << _c;
}
}
I’m going to assume this is a debug function you just forgot to remove. Because it prints the NULs… which don’t really show up… and there is an unnecessary ""
there.
But if this is intended to be a real function then…
- What is the point of making
_c
a const char&
instead of just a char
?
- Rather than hard-coding
std::cout
, you should take a std::ostream&
parameter.
constexpr std::string_view cppStringView(void) const
{
if(this->empty())
{ return std::string_view(""); }
else
{ return std::string_view(this->textBegin(), this->bytesEnd()); }
}
cppStringView()
is kind of an ugly name. What other kind of string view could you be expecting, if not a C++ string view? A Python string view?
Rather than a function, this would be better as a conversion operator:
constexpr operator std::string_view() const& noexcept
{
return {textBegin(), textEnd()};
}
Note the lvalue reference specifier. This is to prevent creating views to temporaries. (And yes, I know this implementation is incorrect, but… that’s the next issue.)
Now, I pointed out the absurdity of mixing iterator types above, but here is where the rubber hits the road. The fact that you have to mix the iterator types here to make this work is a screaming warning alert that something is very, very wrong.
But don’t take my word for it. Try this program:
auto main() -> int
{
using namespace std::literals;
auto const s = "foobar"s;
std::println("initial string = {}", s);
auto const v = LeftZeroPaddedString{s};
std::println("via string view = {}", v.cppStringView());
std::println("via text begin/end = {}", std::string{v.textBegin(), v.textEnd()});
}
Look what I get:
initial string = foobar
via string view = foobar
via text begin/end = fooba
Notice anything wrong?
constexpr bool operator==(const std::string_view& _sv) const
{ return this->cppStringView() == _sv; }
constexpr bool operator==(const LeftZeroPaddedString& _other) const
{ return this->operator==(_other.cppStringView()); }
If you are going to include conversions, you should probably go all the way and define operator<=>
.
constexpr LeftZeroPaddedString& operator=(const LeftZeroPaddedString& lzpd)
{
const std::vector<char>* lzpd_super = &lzpd;
std::vector<char>::assign(lzpd_super->begin(), lzpd_super->end());
return *this;
}
This operator is not necessary. The default is fine.
constexpr void swap(LeftZeroPaddedString& _right)
{
const LeftZeroPaddedString buffer(_right);
_right = *this;
*this = buffer;
}
Do you realize this swap operation is copying the string every time? Like, three copies, just to do a swap?
Do you not know about moving? This should be:
constexpr auto swap(LeftZeroPaddedString& right)
{
auto buffer = std::move(right);
right = std::move(*this);
*this = std::move(buffer);
}
Or, even better:
constexpr auto swap(LeftZeroPaddedString& right) noexcept
{
std::ranges::swap(
static_cast<std::vector<char>&>(*this),
static_cast<std::vector<char>&>(right)
);
}
Note that we can now make this noexcept
, because vector
moves are noexcept
.
Or, even more better, by including the vector
as a data member rather than inheriting:
constexpr auto swap(LeftZeroPaddedString& right) noexcept
{
std::ranges::swap(_data, right._data);
}
In summary:
Inheriting from std::vector
is a terrible idea. There are literally no benefits over including a vector
as a data members, and the costs are that your code is damn near illegible due to all the casting and explicit function calls, and your type is brittle and can silently break if/when vector
adds more member functions in future standards.
Your abstraction is poorly conceived, and nonsensical. Sometimes your type represents just the stuff between the two NULs. Sometimes it includes the two NULs. Sometimes it only includes one of them. This confusion has created actual bugs.
Less code is better. For starters, having to constantly qualify every function call with std::vector<char>::
or casting to std::vector<char>
in one form or another is a sign that you’re doing something wrong. And you should default everything that can be defaulted, and not write any special operations that the implicit defaults work for.
std::string
for this.std::string
can holdNUL
characters in the middle of the string. It’s just tricky to create such strings (because you generally can’t do it from string literals directly). For example \$\endgroup\$