HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Vector Implementation C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationvectorstackoverflow

Problem

I have attempted to implement a similar version of the STL Vector; several functions are missing but I'd like a few words of advice on whether I am indeed on the right track or whether I should change my approach.

My objective is to understand how the vector works from behind the scenes and not to replace or use it for my own applications. I would like to eventually continue this process for all the STL containers.

```
namespace mystl
{
template
class Vector
{
private:
std::unique_ptr values;
std::size_t m_size;
std::size_t m_capacity;
unsigned int m_capacity_inc;

protected:
std::unique_ptr get_values();
public:
/ Type definitions /
typedef T* iterator;
typedef const T* const const_iterator;

/ default constructor /
explicit Vector();
/ filling constructor/
explicit Vector(std::size_t num, const T& val = T());
/ Copy constructor needs to use move since copy is a deleted function in unique_ptr/
explicit Vector(const Vector& v);
/ Move constructor /
explicit Vector(Vector&& v);
/ Initializer list constructor /
explicit Vector(std::initializer_list l);

/ iterator functions /
iterator begin();
iterator end();

/ capacity info functions /
std::size_t size() const;
std::size_t capacity() const;
bool empty() const;

/ modifier functions/
void push_back(T t);
void pop_back();
void clear();

void resize(std::size_t new_size);

/ element access/
T& operator[](int index)
{
return values[index];
}
};

/ move constructor /
template
Vector::Vector(Vector&& v)
{
m_capacity = v.capacity();
m_size = v.size();
m_capacity_inc = m_capacity / 2;
values = (v.get_values());

v.clear();
}

/ initializer list constructor /
template
Vector::Vector(std::initializer_list l)
{
m_size = 0;
m_capacity = l.size();
m_capacity_inc = m_size / 2;
values = std::make_unique(m_capacity);
for (

Solution

Placement New

Your main problem (as described by Jerry Coffin in the comments) is that you are creating objects when you should not be.

If we look at your copy constructor:

Vector::Vector(const Vector& v)
{
    // Here you are allocating several objects of type T
    // This actually calls the default constructor of T
    values = std::make_unique(m_capacity);
    //     Two problems with this:
    //       1) Not all types have default constructors.
    //       2) In the loop below you are then calling the assignment operator.
    //          Which means you are doing twice the work you need to do.

    for (auto i = 0; i < m_size; ++i)
    {
        values[i] = v.values[i];
    }
}


Next lets look at re-size

void Vector::resize(std::size_t new_size)
{
    // The problem here.
    // If you re-size to double the number of elements (say an extra 'n')
    //    You are creating n elements that you don't need.
    //    Especially if the constructor of `T` is expensive of uses
    //    precious resources you don't want to do that. You do want to reserve
    //    the space but you don't actually want to construct the object
    //    until the size is correct
    //
    //    And then you want to construct the object in place not call the
    //    constructor then perform a copy construction on top of that.
    auto new_array = std::make_unique(new_size);
}


This is where placement new comes in.

// Create your buffer where the data is going to be placed.
 // But do not do any work on construction.
 char*  data = new char[sizeof(T) * countOfObjects];

 // If you want to copy data into the buffer you use placement new.
 T*  dst = reinterpret_cast(data);
 for(auto i = 0;i < src.size(); ++i) {
     new (dst + i) T(src[i]);    // Copy construct using placement new.
 }


Memory Management

Normally there are two ways to do memory management. Smart pointers and containers. These are complimentary methods and you don't usually mix them together.

So I would expect a vector to do its own memory management and not delegate this work to a smart pointer (though you can).

Code Snippets

Vector<T>::Vector(const Vector<T>& v)
{
    // Here you are allocating several objects of type T
    // This actually calls the default constructor of T
    values = std::make_unique<T[]>(m_capacity);
    //     Two problems with this:
    //       1) Not all types have default constructors.
    //       2) In the loop below you are then calling the assignment operator.
    //          Which means you are doing twice the work you need to do.

    for (auto i = 0; i < m_size; ++i)
    {
        values[i] = v.values[i];
    }
}
void Vector<T>::resize(std::size_t new_size)
{
    // The problem here.
    // If you re-size to double the number of elements (say an extra 'n')
    //    You are creating n elements that you don't need.
    //    Especially if the constructor of `T` is expensive of uses
    //    precious resources you don't want to do that. You do want to reserve
    //    the space but you don't actually want to construct the object
    //    until the size is correct
    //
    //    And then you want to construct the object in place not call the
    //    constructor then perform a copy construction on top of that.
    auto new_array = std::make_unique<T[]>(new_size);
}
// Create your buffer where the data is going to be placed.
 // But do not do any work on construction.
 char*  data = new char[sizeof(T) * countOfObjects];


 // If you want to copy data into the buffer you use placement new.
 T*  dst = reinterpret_cast<T*>(data);
 for(auto i = 0;i < src.size(); ++i) {
     new (dst + i) T(src[i]);    // Copy construct using placement new.
 }

Context

StackExchange Code Review Q#116377, answer score: 6

Revisions (0)

No revisions yet.