An alternative vector

Based on this question (as a starting point) and a couple of other questions about rewriting vector in C++, I am planning to write another blog article to show the best way (or more precisely, the common pitfalls that most people fall into).

Before I start the blog article just wanted feedback to make sure I am not doing anything stupid.

```
#include
#include
#include

namespace ThorsAnvil
{
template
class V
{
std::size_t capacity;
std::size_t length;
T* buffer;

void makeSpaceAvailable()
{
if (length == capacity)
{
int newCapacity = std::max(10ul, capacity) * 1.62;
V tmp(newCapacity);
std::move(buffer, buffer + length, tmp.buffer);
tmp.swap(*this);
}
}
void validateIndex(std::size_t size)
{
if (size >= length)
{
std::stringstream message;
message (::operator new(sizeof(T) * capacity)))
{}
V(V const& copy)
: capacity(copy.capacity)
, length(copy.length)
, buffer(static_cast(::operator new(sizeof(T) * capacity)))
{
std::copy(copy.buffer, copy.buffer + copy.length, buffer);
}
V& operator=(V value) // pass by value so this is a copy.
{
value.swap(*this); // Copy and Swap idiom
return *this;
}
V(V&& move) noexcept
: capacity(0)
, length(0)
, buffer(nullptr)
{
move.swap(*this); // Move just swaps content.
}
V& operator=(V&& value) noexcept
{
value.swap(*this);
return *this;
}
~V() noexcept
{
for(int loop = 0; loop (u));

Ok, you asked for it, so let's see what we can find.

• There's seriously no reason for your default-constructor or move-constructor to ever throw an exception, as they don't actually need to aquire any resources (buffer != nullptr is already not an invariant anyway).

Actually, a move-constructor which can throw is nigh useless.

Thus, in C++1z they will be marked noexcept, if neccessary dependent on allocator default-constructor.

-
I would change makeSpaceAvailable in two ways:

• Make it accept the new capacity, so one can pre-reserve as much as needed.

• Split out the part actually doing the hard work of resizing, for reuse and so the rest can be easily inlined.

• For validateIndex, I would also split out the error-case into its own method, so the rest is more likely to be inlined.

• Your copy-constructor depends on the element-type having a trivial default constructor.

Otherwise, treating raw memory as initialized objects would invoke UB.

-
You special-cased move-assignment, so the assignment-operator accepting by value only handles copying. Considering you thus forego the advantage of only having one of them, consider optimizing that one too, by making the copy in the function instead:

V& operator=(V&& value) noexcept; // See question
V& operator=(const V& value) { swap(V(value)); return *this; }

-
The destructor is ... interesting. You are catching exceptions if thrown by the individual destructors or the deallocation-function, and then silently swallow them.

Which is at best harmless (if the element-type's destructor is well-behaved and thus cannot throw, as is the deallocation-function).

At worst, you masked a catastrophic error which will now wipe out much more, so don't do that.

Simply remove all exception-handling there, and let the language call std::terminate due to trying to propagate an exception from a noexcept-function.

• I suggest you delegate all the hard work from the non-const version of at to the const one, as the const-qualifiers are the only difference.

• As you have emplace, there's no sense in re-implementing that functionality for push. Just delegate.

• You decided to define the behavor of pop on underflow. That's a valid decision, though it certainly adds some overhead...

• There are quite some members which could be noexcept-qualified.

StackExchange Code Review Q#121127, answer score: 6