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

Template vector class

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

Problem

I am trying to implement a vector-like container of my own (just to gain a better understanding of how std::vector works under the hood). I've been using this as a reference.

Although I've succeeded in implementing most of the interface (only the parts I use the most), I'm still uncertain whether:

  • There are no memory leaks.



  • All the routines associate with std::vector with respect to performance and space-usage.



Please review it to see if it can be improved. Any suggestions related to my commenting style are also welcome!

```
size_t nearest_power_of_2(size_t n) // Return the nearest( and strictly greater than ) number to "n" which is a power of 2..
{
int count=0;
while(n)
{
n>>=1;
count++;
}
return 1ULL
class myvector
{
T* vector_pointer; // A pointer pointing to the start of the dynamic array...
size_t vector_size,vector_capacity;

public:
myvector():vector_pointer(NULL),vector_size(0),vector_capacity(0) {} // Default Constructor

myvector(const myvector& other):vector_pointer(NULL),vector_size(0),vector_capacity(0) // Copy Constructor
{
vector_pointer=new T[other.capacity()];
memcpy(vector_pointer,&other[0],sizeof(T)*other.size());
vector_size=other.vector_size;
vector_capacity=other.vector_capacity;
}

~myvector() // Destructor
{
delete[] vector_pointer;
}

myvector& operator =(myvector other) // Assignment operator
{
swap(*this,other);
return *this;
}

void resize(const size_t &newsize) // Change the size of the vector exactly to "newsize"..
{
if(newsizevector_capacity)
{
reserve(newsize);
}
vector_size=newsize;
}

void reserve(size_t newcapacity) // Change the capacity of the vector to be at least equal to "newcapacity"
{
newcapacity=nearest_power_of_2(newcapacity); // Keep the capacity of the vector as a power of 2 to av

Solution

You can do this with maths rather than a loop:

size_t nearest_power_of_2(size_t n)  // Return the nearest( and strictly greater than ) number to "n" which is a power of 2..


Couple of issues with the copy constructor.

myvector(const myvector& other):vector_pointer(NULL),vector_size(0),vector_capacity(0)  // Copy Constructor
{
    // Don't assign to this object until you
    // know that no exceptions can be thrown.
    // This means all memory allocation and
    // object copying must be completed first.
    vector_pointer=new T[other.capacity()];

    // You can use this for POD types.
    // But any non POD types must be copied using their copy constructor.
    memcpy(vector_pointer,&other[0],sizeof(T)*other.size());

    vector_size=other.vector_size;
    vector_capacity=other.vector_capacity;
}


Similar issues with resize()

// You can use this for POD types.
        // But any non POD types must be copied using their copy constructor.
        memcpy(temp,vector_pointer,sizeof(T)*newsize);


Making the size smaller is easier. You don't need to re-allocate. You can just reduce the size value and manually call the destructor on all the objects that don't exist anymore.

Same but slightly different in reserve()

// The trouble with use T as the underlying type.
        // As that this call will call the constructor on all objects
        // upto `capacity`. You don't want that you only want to call
        // the constructor on values upto `newcapacity`
        T* temp=new T[newcapacity];

        // You can use this for POD types.
        // But any non POD types must be copied using their copy constructor.
        memcpy(temp,vector_pointer,sizeof(T)*vector_capacity);


Your push back is very inefficient:

void push_back(const T &val)  // Add a new element of value "val" at the end of the vector..
{
    if(vector_capacity<=vector_size)
        // As noted calls copy constructor
        reserve(vector_capacity);

    // Now using the assignment operator.
    // To copy over object that you just fully initialized.
    vector_pointer[vector_size++]=val;

    // A better solution is not to construct in the copy constructor.
    // Then here use placement new to create the object in place
}


The pop back needs work

void pop_back()  // Remove the last element of the vector..
{
    if(vector_size)
        --vector_size;

    // RAII is an important concept in C++
    // When you pop something out of your vector it should no longer exist.
    // This means you need to call the destructor for the object.
    // Here that means manually calling the destructor.

}  //  Doesn't actually deallocate the element block (for performance)..


More times when destructor needs to be called.

void erase(const size_t& pos)  // Erase the element at "pos"..
void clear()

// For the elements passed the end of the valid vector (vector_size)
// You need to call the destructors of these elements as they no longer exist.


Fix your output operator:

templatestd::ostream& operator  &a)  // Overloaded output operator to display the contents of a vector..
{
    for(size_t i=0; i<a.size(); i++)
    {
        // Should be `out`
        std::cout<<a[i]<<" ";
    }
    return out;
}


For loops like this use the standard algorithms:
Personally I don't see the point. I can achieve the same effect in one line:

std::copy(a.begin(), a.end(), std::ostream_iterator(out, " "));


Simple Vector:

``
template
class LokiVector
{
std::size_t size;
std::size_t capacity;
std::unique_ptr data; // Use char so we don't worry about
// constructor/destructor issues.

void init(T const& value)
{
public:
LokiVector(std::size_t startSize = 0, T const& defaultValue = T())
: size(startSize)
, capacity(std::min(startSize, 16))
, data(new char[sizeof(T) * capacity])
{
T* localData = reinterpret_cast(data.get());
for(int loop = 0;loop (data.get());
for(int loop = 0;loop (data.get());
for(int loop = 0;loop (data.get());
new (&localData[size]) T(value);
++size;
}

void pop_back()
{
if (size > 0)
{
--size;
T* localData = reinterpret_cast(data.get());
localData[size].~T();
}
}

void reserve(std::size_t min)
{
if (min tmp_data(new char[sizeof(T) * tmp_capacity);
T* localData = reinterpret_cast(data.get());
T* tmpLocal = reinterpret_cast(tmp_data.get());

for(int loop = 0;loop (data.get());
return localData[index];
}
T const& operator[] (std::size_t index) const
{
return const_cast(*this)[index];
}
}
`

Code Snippets

size_t nearest_power_of_2(size_t n)  // Return the nearest( and strictly greater than ) number to "n" which is a power of 2..
myvector(const myvector& other):vector_pointer(NULL),vector_size(0),vector_capacity(0)  // Copy Constructor
{
    // Don't assign to this object until you
    // know that no exceptions can be thrown.
    // This means all memory allocation and
    // object copying must be completed first.
    vector_pointer=new T[other.capacity()];

    // You can use this for POD types.
    // But any non POD types must be copied using their copy constructor.
    memcpy(vector_pointer,&other[0],sizeof(T)*other.size());


    vector_size=other.vector_size;
    vector_capacity=other.vector_capacity;
}
// You can use this for POD types.
        // But any non POD types must be copied using their copy constructor.
        memcpy(temp,vector_pointer,sizeof(T)*newsize);
// The trouble with use T as the underlying type.
        // As that this call will call the constructor on all objects
        // upto `capacity`. You don't want that you only want to call
        // the constructor on values upto `newcapacity`
        T* temp=new T[newcapacity];

        // You can use this for POD types.
        // But any non POD types must be copied using their copy constructor.
        memcpy(temp,vector_pointer,sizeof(T)*vector_capacity);
void push_back(const T &val)  // Add a new element of value "val" at the end of the vector..
{
    if(vector_capacity<=vector_size)
        // As noted calls copy constructor
        reserve(vector_capacity);

    // Now using the assignment operator.
    // To copy over object that you just fully initialized.
    vector_pointer[vector_size++]=val;

    // A better solution is not to construct in the copy constructor.
    // Then here use placement new to create the object in place
}

Context

StackExchange Code Review Q#29331, answer score: 8

Revisions (0)

No revisions yet.