patterncppModerate
Reimplementation of C++ vector
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
```
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);
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
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:
You are only allocating size space. But your capacity is 20 bytes larger.
Why 20 bytes? Should this not be 20 objects
BUG:
You use the allocator construct routines when you are doing
Thus in
This line:
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
I think you mean
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.
__ 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.