Custom STL Vector in C++

This is my first time I tried to implement a custom STL vector with iterator. I have some questions:

• What are better ways to write an iterator?

• I know that there is a const iterator so in my case I need to create a second class for iterator? Is there any approachable way to not repeat yourself with second (and others) iterators?

• Why in vector there are a lot of functions which can cause undefined behaviour?

For example : method back and I quote : "Calling back on an empty container is undefined" but why? Why in STL there is no place for exceptions? I read some explanations but it was mainly "c++ it's all about speed", throwing exceptions and making code probably safer is such a big deal (in reference to time?)

Vector.h

```
#ifndef VECTOR_TEMPLATE_VECTOR_H
#define VECTOR_TEMPLATE_VECTOR_H

#include "Iterator.h"

template
class Vector {
private:
DT *tab;
int sizeVector;
int capacityVector;
public:
Vector() : tab(nullptr), sizeVector(), capacityVector() {}
explicit Vector(int size);
Vector(int size, int value);
Vector(const std::initializer_list & v);
Vector(const Vector & v);
Vector(Vector && v);

Vector & operator=(const Vector &v);
Vector & operator=(Vector &&v);

~Vector();

bool operator==(const Vector & v);
bool operator!=(const Vector & v);

int capacity();
int size();

void push_back(DT & v);

void pop_back();

void swap(Vector & other);

DT & front();
const DT & front() const;

const DT & back() const;
DT & back();

DT &operator[](int size);
const DT &operator[](int size) const;

void assign(int count, const DT &value);

void resize(int count);

friend class Iterator;
typedef class Iterator iterator;

Iterator erase(Iterator pos);
Iterator erase(Iterator first, Iterator last);
Iterator insert(Iterator pos, const DT & value);
void insert(iterator pos, int count, const DT & value);

Iterator end();

Good first try.

Rule of 5 done. Excellent.

The one thing to note is that you should probably mark the move operators as noexcept this will provide extra optimization opportunities to the compiler when your class is used with standard containers (or within classes that are optimized for movable objects). Note: The swap operator is also usually noexcept.

You missed a couple of const functions, though in general you did rather well.

// These should all be const members.
bool operator==(const Vector & v);
bool operator!=(const Vector & v);
int capacity();
int size();

Missed moving and emplacing elements into vector:

void push_back(ST&& move);
template
void emplace_back(A&&... args);

Design:

The problem with your vector is that you initialize all the elements in the vector. This has two nasty side effects:

• This means that your template class DT must be default constructible.

• If DT is expensive to construct you are doing wasted work.

The point of capacity. Is that it tracks the number of uninitialized members of the vector. You only construct these members as you use them (thus they can be constructed with parameters) and you don't pay for their construction if you don't need them.

// Allocate memory
char* data = new char[sizof(DT) * capacityVector];

// Add a new element to the end.
DT* location = &(reinterpret_cast(data)[sizeVector]);
new (location) DT(copy);
++sizeVector;

// Remove an element from the end.
DT* location = &(reinterpret_cast(data)[sizeVector-1]);
location->~DT();
--sizeVector;

Code Review

Issues:

Your copy assignment is not exception safe. The general rule is that you should not release your resources until you have the replacement ready. Then you should swap the resources.

template
Vector &Vector::operator=(const Vector &v) {
    if (this == &v)
        return *this;

    delete[] tab;

    // What happens if there is an exception after this point?
    // If the following `new` throws a Bad Alloc this object is
    // left in a bad state with (the tab pointer is currently bad).

    sizeVector = v.sizeVector;
    capacityVector = v.capacityVector;
    tab = new DT[capacityVector];

    for (int i = 0; i < capacityVector; i++)
        tab[i] = v.tab[i];

    // you forgot the return
    return *this;
}

You should write it like this:

template
Vector &Vector::operator=(const Vector &v) {
    if (this == &v)
        return *this;

    int newSizeVector     = v.sizeVector;
    int newCapacityVector = v.capacityVector;
    DT* newTab            = new DT[capacityVector];

    for (int i = 0; i < newCapacityVector; i++)
        newTab[i] = v.tab[i];

    // You have now done all the dangerious work.
    // so swap the state in an excetion safe way.

    swap(newSizeVector,     sizeVector);
    swap(newCapacityVector, capacityVector);
    swap(newTab,            tab);

    // Now you can release the resources safely.
    delete[] newTab;

    return *this;
}

But this is all done much tidier when you use the copy and swap Idiom. The constructor/destructor of the temp object handles or the resource management for you.

template
Vector &Vector::operator=(const Vector &v) {

    Vector copy(v);
    copy.swap(*this);
    return *this;
}

Simplify

The move operators are usually just implemented as swap operations. This delays destruction until the moved object is destroyed which is a potentiall optimization if it is not destroyed but re-used.

template
Vector::Vector(Vector &&v)
    : sizeVector(v.sizeVector)
    , capacityVector(v.capacityVector)
{
    tab = v.tab;       // Not sure why you did this in the body 
                       // and the other two in the initializer list.
    v.tab = nullptr;
    v.sizeVector = 0;
    v.capacityVector = 0;
}
// Simpler to write as:
template
Vector::Vector(Vector &&v)
    : sizeVector(0)
    , capacityVector(0)
    , tab(nullptr)
{
    v.swap(*this);
}

template
Vector &Vector::operator=(Vector &&v) {
    v.swap(*this);    
    return *this;
}

Swap

There is already a standard swap function. You should use it (like any standard algorithm when you can). It will simplify your code and express intent much more clearly.

template
void Vector::swap(Vector &other) {
    int tempSize, tempCapacity;
    DT *tempTab;

    tempTab = other.tab;
    tempSize = other.sizeVector;
    tempCapacity = other.capacityVector;

    other.tab = tab;
    other.sizeVector = sizeVector;
    other.capacity = capacityVector;

    sizeVector = tempSize;
    capacityVector = tempCapacity;
    tab = tempTab;
}

// Simpler as:
template
void Vector::swap(Vector &other) noexcept {
    using std::swap;
    swap(tab,           other.tab);
    swap(sizeVector,    other.sizeVector);
    swap(capacityVector,other.capacityVector);
}

Question


What are better ways to write an iterator?

I think your iterator is basically fine. Though to make them work correctly with the iterator traits you should defin

StackExchange Code Review Q#154008, answer score: 4