STL vector implementation

I've implemented a simple vector-like structure. I would appreciate all criticism relevant to code, style, flow, camelCase vs underscore, and so forth.

```
template
class Vector {
public:

typedef T* Iterator;

Vector();
Vector(unsigned int size);
Vector(unsigned int size, const T & initial);
Vector(const Vector& v);
~Vector();

unsigned int capacity() const;
unsigned int size() const;
bool empty() const;
Iterator begin();
Iterator end();
T& front();
T& back();
void push_back(const T& value);
void pop_back();

void reserve(unsigned int capacity);
void resize(unsigned int size);

T & operator[](unsigned int index);
Vector & operator = (const Vector &);
void clear();
private:
unsigned int _size;
unsigned int _capacity;
unsigned int Log;
T* buffer;
};

template
Vector::Vector() {
_capacity = 0;
_size = 0;
buffer = 0;
Log = 0;
}

template
Vector::Vector(const Vector & v) {
_size = v._size;
Log = v.Log;
_capacity = v._capacity;
buffer = new T[_size];
for (unsigned int i = 0; i
Vector::Vector(unsigned int size) {
_size = size;
Log = ceil(log((double) size) / log(2.0));
_capacity = 1
bool Vector:: empty() const {
return _size == 0;
}

template
Vector::Vector(unsigned int size, const T& initial) {
_size = size;
Log = ceil(log((double) size) / log(2.0));
_capacity = 1
Vector& Vector::operator = (const Vector & v) {
delete[] buffer;
_size = v._size;
Log = v.Log;
_capacity = v._capacity;
buffer = new T [_capacity];
for (unsigned int i = 0; i
typename Vector::Iterator Vector::begin() {
return buffer;
}

template
typename Vector::Iterator Vector::end() {
return buffer + size();
}

template
T& Vector::front() {
return buffer[0];
}

template
T& Vector::back() {
return buffer[_size - 1];
}

template
void Vector::push_back(const T & v) {
/*
Incidentally, one common w

Your code is C++03 like (ie there are no move constructors or move assignment operators). You should definitely think about updating your class to be move aware.

The first thing about the interface is that the assignment operator is not close to the constructors. Since these are highly linked I like to place the assignment operators very close to the constructors (see rule of three/five).

Using _ as a prefix is a bad idea. You don't break any rules (but was this on purpose or just an accident?). The rules are sufficiently complex that prefix _ on identifiers is a bad idea for user space code. Others have suggested a prefix like m_ personally I think that is old school advice. As long as the naming is clear there should be no issues.

My personal convention with identifiers is that anything that can be an object starts with a lowercase letter. Anything that is a type begins with an uppercase letter. Others will disagree with this convention as the standard does not follow it and this can lead to other conflicts.

Here is one of those exceptions to my rules.

typedef T* Iterator;

You should probably have iterator with a lowercase i. This is because a lot of the algorithms will use the type.

Good start.

Iterator begin();
    Iterator end();
    T& front();
    T& back();
    T & operator[](unsigned int index);

But you will need const versions of all these (and a const_iterator type) to be a fully compliant container.

You are going to have a hard time making this work if the buffer is of type T. Every time you expand the buffer all the elements in the buffer will be initialized with T constructor. For int this is not a problem. But if T has a non trivial constructor then you are going to pay a heavy price initializing elements that may never be used.

T* buffer;

Really the buffer should be something that does not have a constructor.

char* buffer;

Prefer the initializer list.

template
Vector::Vector() {
    _capacity = 0;
    _size = 0;
    buffer = 0;
    Log = 0;
}

// This will also warn about out of order initialization.
// Not a big thing but it keeps your thinking logical.

template
Vector::Vector()
    : _size(0)
    , _capacity(0)
    , Log(0)
    // Personally I would always allocate some space for the buffer
    // even if the size is zero. This way I do not have to worry about
    // the special case of buffer ever being a NULL pointer.
    //
    // Having a special case of NULL will affect all the methods and make
    // the code much more complex. Reduce code complexity means easier to
    // maintain code.
    //
    , buffer(new char[sizeof(T) * capacity])
    // Note this assumes buffer is (char* as described above)
{}

Again. Use an initializer list.

template
Vector::Vector(const Vector & v) {
    _size = v._size;
    Log = v.Log;
    _capacity = v._capacity;

Do you really mean to allocate _size memebers to the buffer. Should this not be _capacity?

buffer = new T[_size];

    for (unsigned int i = 0; i < _size; i++)
        buffer[i] = v.buffer[i];
}

Also use the internal algorithms to help you.

template
Vector::Vector(Vector const& v)
 // Note. I put cost here   ^^^^^   most of the time it does not matter.
 // There is one corner case where it does.
    : _size(v._size)
    , _capacity(v._capacity)
    , Log(v.Log)
    , buffer(new char[sizeof(T) * _capacity])
{
    for (unsigned int i = 0; i < _size; i++)
    {
        // Now you can use placement new to copy the elements from
        // source into the current vector.
        new (buffer + sizeof(T) * i) T(v[i]);
    }
}

Again use initializer list.

template
Vector::Vector(unsigned int size) {
    _size = size;

This seems to be a constant. I would just make this a constant of the class and calculate it once by hand. The use of these maths functions seems a bit like overkill to me.

Log = ceil(log((double) size) / log(2.0));
    _capacity = 1 << Log;
    buffer = new T[_capacity];
}

Personally one liners like this.

template 
bool Vector:: empty() const {
    return _size == 0;
}

I just put inline into the class.

The assignment operator problem has been covered by @liv902

template
Vector& Vector::operator = (const Vector & v) {
    delete[] buffer;
    _size = v._size;
    Log = v.Log;
    _capacity = v._capacity;
    buffer = new T [_capacity];
    for (unsigned int i = 0; i < _size; i++)
        buffer[i] = v.buffer[i];
    return *this;
}

Basically you should not change the state of your object until you have guranteed that there are no expceptions can be thrown and leave your object in an incosistent state. This means your assignemnt should proceed in three stages.

• Copy the state of the src class into temporary objects.

This is because making the copy can throw an exception and thus you should not put your object into a state wheere it can be invalid.

• Swap the state of you object with the temp

StackExchange Code Review Q#60484, answer score: 23