Remaking the C++ std::vector class

I haven't really programmed in C++ for about a year, and realised that I should get back into it, and tried my abilities out by remaking the STD vector class. However, my C++ is a bit rusty at the moment, and was wondering if I have made many mistakes in my implementation.

```
# ifndef VECTOR_H
# define VECTOR_H
# include

namespace test
{
template

class vector;

template

class vector_alloc_types
{
public:
typedef typename A::value_type value_type;
typedef typename A::size_type size_type;
typedef typename A::reference reference;
typedef typename A::const_reference const_reference;
typedef typename A::pointer iterator;
typedef typename A::const_pointer const_iterator;
typedef typename A::const_pointer const_pointer;
typedef A allocator_type;
};

template

class vector_base
{
friend class vector;
public:
typedef typename A::pointer pointer;
vector_base()
{
vm_begin = pointer();
value_end = pointer();
memory_end = pointer();
}
private:
pointer vm_begin, value_end, memory_end;
};

template
>
class vector
: public vector_alloc_types,
public vector_base
{
public:
typedef vector my_T;
typedef vector_base my_base;

vector()
: my_base()
{
}

vector(my_T const &rhs)
{
if (allocate(rhs.size()))
{
try
{
this->value_end = std::uninitialized_copy(rhs.vm_begin, rhs.value_end, this->vm_begin);
}
catch (...)
{
kill();
throw;
}
}
}

vector(pointer first, poi

This is certainly not a bad attempt at all. I especially appreciate the detail to the correct use of uninitialized memory.
Indentation

It is great that you are using a consistent indentation style. However, it is an uncommon one, which I think will be off-putting for many C++ developers. A more common style would be:

template 
class vector_base
{
    friend class vector;
public:
    typedef typename A::pointer pointer;
    …
};

Prefer using over typedef

using has some advantages over typedef, in particular:

• The name is clearly separated from the type it aliases (which is especially nice if the type is an array, a function pointer, or combination thereof).

• You can also use it without assigning a new name to something, in which case you bring a name from one namespace into another.

It allows you to write:

template 
struct vector_alloc_types
{
    using typename A::value_type;
    …
    using typename A::const_pointer;
    using allocator_type = A;
};

Don't use inheritance

I fail to see why you are defining vector_alloc_types and vector_base, and have vector inherit from it. Nothing is gained from it, and now you have to worry about:

• Making vector_base a friend of vector.

• Does public inheritance expose any members from the base classes that you don't want exposed in vector?

• Does an empty class like vector_alloc_types take up space? See empty base optimization.

I would avoid all that by just keeping everything in a single class.
Make use of default member initialization

You can avoid having to explicitly initialize member variables in the constructors if you use default member initialization instead, like so:

template >
class vector
{
    using pointer = …;
    …
    pointer vm_begin{};
    pointer value_end{};
    pointer memory_end{};

public:
    vector() = default;
    vector(pointer first, pointer last)
    {
        if (allocate(std::distance(first, last))) {
            …
        }
    }
    …
};

Pay closer attention to the interface of std::vector

If you want to remake std::vector, pay close attention to its interface and copy it exactly. Your vector differs in several cases, including:

• Missing constructor overloads. Especially since you did add a template parameter for the allocator, it is weird to not see a constructor that takes an allocator object as a parameter.

• Missing move constructor and assignment operator. These can easily be implemented in terms of swap().

• Assignment operators should return a reference to *this when possible. Almost any class in the standard library does this, and allows you to write code like a = b = c and if ((a = b)).

• Missing empty(), max_size(), emplace_back() and relational operators.

• Incorrect type aliases. For example your value_type is equal to A::value_type, but in std::vector it is equal to T. Don't assume those are the same; this is only guaranteed for the default allocator.

Unnecessary use of this->

It is almost never necessary to use this-> in C++. Omit it where possible, it just adds noise to the code.
Don't kill() in the constructors

Your kill() will try to destroy the objects in the vector when functions like std::uninitialized_copy() fail. However, if the latter fails, it is likely one or more objects were not constructed. Calling a destructor on an object that was not properly constructed has undefined behavior. Also, std::uninitialized_copy() and friends will already destroy any objects they constructed for you if they throw an exception.
Memory leaks

In erase(), if you erase an element from the middle, you will make a copy of the whole vector minus the erased element, and then you assign() that copy to the current vector. However, you never free the memory from the copy.
You make the same mistake in more functions.

A simple fix would be to use the same technique as you used in push_front(): create a temporary vector, copy things into that, and then swap() it.
Inefficient operations

Erasing an element should not require you to copy all of the other elements; you only need to move the elements after the erased element one position towards the front.

The exact amount of optimizations you can do here depends on the exception safety guarantees you want to have. If calling erase() throws, should the contents of the vector be the same as before the call to erase()? If so, then if T doesn't make any guarantees about exception safety of coping or moving its elements, then you have no option but to make a full copy of the vector first, and then swap()ing.

But pop_back() is a special case where you know you don't need to copy anything at all; you just need to destroy the last element.

In push_back(), if there was no spare capacity, you only resize the storage to hold one more element. This is very inefficient: consider filling a vector by repeatedly calling push_back(): each time it has to allocate new

StackExchange Code Review Q#82906, answer score: 2