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

Reimplementation of C++ vector

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

Problem

I previously attempted to make a C++ vector here, yet it was not very successful. Now I have made a basic reimplementation of it, so I'm checking that it is fine, and that I will not have to re make it again.

Note: I also implemented an allocator class, but it works the same as the std:: equivalent.

```
template > class vector
{
public:
typedef vector _Myt;
typedef _Ty value_type;
typedef _Alloc allocator_type;
typedef value_type *pointer;
typedef const value_type *const_pointer;
typedef value_type *iterator;
typedef const value_type *const_iterator;
typedef value_type &reference;
typedef const value_type &const_reference;
typedef std::size_t size_type;
typedef std::ptrdiff_t difference_type;

vector()
: __size(0), __capacity(20), __data(_Alloc().allocate(20))
{
}

vector(const _Myt &_Rhs)
: __size(_Rhs.__size), __capacity(_Rhs.__size + 20), __data(_Alloc().allocate(_Rhs.__size))
{
int count = 0;
for (iterator i = &_Rhs.__data[0]; i != &_Rhs.__data[_Rhs.__size]; ++i, ++count)
{
_Alloc().construct(&__data[count], *i);
}
}

~vector()
{
if (!empty())
{
for (iterator i = begin(); i != end(); ++i)
{
_Alloc().destroy(i);
}
_Alloc().deallocate(__data, __capacity);
}
}

_Myt &push_back(const value_type &_Value)
{
if (++__size >= __capacity)
{
reserve(__capacity * 2);
}
_Alloc().construct(&__data[__size - 1], _Value);
return *this;
}

_Myt &push_front(const value_type &_Value)
{
if (++__size >= __capacity)
{
reserve(__capacity * 2);
}
if (!empty())
{
iterator _e = end(), _b = begin();
std::uninitialized_copy(_e - 1, _e, _e);
++_e;
std::copy_backward(_b, _e - 2, _e - 1);

Solution

STOP USING __ its reserved for the implementation.

Basically stop using underscore until you know the rules.
This code is basically all broken because of this. You code technically is all undefined.

We told you this before. STOP IT.

BUG:

vector(const _Myt &_Rhs)
    : __size(_Rhs.__size), __capacity(_Rhs.__size + 20), __data(_Alloc().allocate(_Rhs.__size))


You are only allocating size space. But your capacity is 20 bytes larger.

Why 20 bytes? Should this not be 20 objects 20 * sizeof(_Ty)

BUG:

You use the allocator construct routines when you are doing placement new. This means the space has not been constructed before. If the location has already got an element in it you must copy over it using the copy constructor.

Thus in

_Myt &push_front(const value_type &_Value)


This line:

_Alloc().construct(&__data[0], _Value);


You are constructing into location 0 a new object without calling the old objects destructor. If your type is anything non trivial this will screw up any memory management.

BUG

bool empty()
{
    return !__data;  // Both constructors allocate memory.
}                    // So this will never be true.


I think you mean

return size != 0;


You have a potential leak situation in your reserve().

You provide the strong exception gurantee. BUT if the copy fails (with an exception) then you will leak the buffer. You need to hold the buffer in a smart pointer until you swap so that if there is an exception you gurantee that you deallocate the unused buffer.

Code Snippets

vector(const _Myt &_Rhs)
    : __size(_Rhs.__size), __capacity(_Rhs.__size + 20), __data(_Alloc().allocate(_Rhs.__size))
_Myt &push_front(const value_type &_Value)
_Alloc().construct(&__data[0], _Value);
bool empty()
{
    return !__data;  // Both constructors allocate memory.
}                    // So this will never be true.
return size != 0;

Context

StackExchange Code Review Q#43136, answer score: 11

Revisions (0)

No revisions yet.