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

Implementation of std::vector class

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

Problem

I decided to implement the std::vector class in C++, and was wondering if I had done everything correctly. When I mean 'correctly', I mean that the code is efficiently written and done properly. I have tested all the functions using various tests and they seem to output correct results.

I have not included every function in the std class: the main ones that I have not included are assign/insert and I have not implemented a reverse_iterator, although I have made my own iterator and const_iterator class.

```
# include
// I included the below two headers for my tests
# include
# include

namespace mynamespace
{
template
class vector;

template
class vector_const_iterator;

template
class vector_iterator
{
public:
typedef vector_iterator this_t;
typedef typename Vec_T::value_type T;

friend Vec_T;

vector_iterator(T *ptr_, Vec_T const &cont)
: container(cont),
ptr(ptr_)
{
}

vector_iterator(this_t const &rhs)
: container(rhs.container),
ptr(rhs.ptr)
{
}

vector_iterator(vector_const_iterator const &rhs)
: container(rhs.container),
ptr(rhs.ptr)
{
}

~vector_iterator()
{
}

bool operator!=(this_t const &rhs) const
{
return (ptr != rhs.ptr);
}

bool operator!=(T *ptr_) const
{
return (ptr != ptr_);
}

bool operator==(this_t const &rhs) const
{
return (ptr == rhs.ptr);
}

bool operator==(T *ptr_) const
{
return (ptr == ptr_);
}

bool operator>(this_t const &rhs) const
{
return (ptr > rhs.ptr);
}

bool operator>(T *ptr_) const
{
return (ptr > ptr_);
}

bool operator>=(this_t const &rhs) const
{
return (ptr >= rhs.ptr);
}

bool operator>=(T *ptr_) const
{
return (ptr >= ptr_);
}

bool operat

Solution

There's a rather large amount of code here, so I'm just going to make some comments as I find things.

void reallocate(unsigned size)
    {
    allocator_t al;
    while (size == capacity())
        {
        size *= 2;
        }


This looks like it might be a bug, did you mean to do while size <= capacity instead?

Additionally you probably want to check for overflow here too otherwise bad things will happen, make sure size*2 actually fits into an unsigned.

Also I might be biased slightly because I've been using Python a lot lately but I particularly dislike declaring variables like this:

pointer new_memstart = al.allocate(size), new_seqend;


Putting one variable per line I find much nicer.

One thing I notice here is duplicated code for the operators in a few places. Specifically I'd implement as many operators as possible in terms of other operators. For example:

bool operator==(this_t const &rhs) const
    {
    return (ptr == rhs.ptr);
    }

bool operator!=(this_t const &rhs) const
    {
    return (ptr != rhs.ptr);
    }


would become:

bool operator==(this_t const &rhs) const
    {
    return (ptr == rhs.ptr);
    }

bool operator!=(this_t const &rhs) const
    {
    return !operator==(rhs);
    }


You can do this for a bunch of the operators, which will help the maintainability of the code.

Now while the following is not exactly duplicated:

bool is_valid_ptr() const
    {
    return (ptr >= container.mem_start
        && ptr = container.mem_start
        && ptr <= container.seq_end);
    }


It looks very similar. A comment explaining why these 2 seemingly similar functions exist and when to which version would be good. While most of the code seems self-explanatory the places that are less obvious would greatly benefit from some documentation.

EDIT as per the comments:

I might be inclined to rewrite the pointer validity check functions as follows:

/** Check if the pointer is a valid location in the container */
bool is_valid_ptr() const
    {
    return (ptr >= container.mem_start
        && ptr < container.seq_end);
    }

/** Check if the pointer is a valid location in the container or is the end pointer */
bool is_valid_ptr_or_end() const
    {
    return (is_valid_ptr() || ptr == container.seq_end);
    }


This change makes it a lot clearer to the reader what is going on here.

Code Snippets

void reallocate(unsigned size)
    {
    allocator_t al;
    while (size == capacity())
        {
        size *= 2;
        }
pointer new_memstart = al.allocate(size), new_seqend;
bool operator==(this_t const &rhs) const
    {
    return (ptr == rhs.ptr);
    }

bool operator!=(this_t const &rhs) const
    {
    return (ptr != rhs.ptr);
    }
bool operator==(this_t const &rhs) const
    {
    return (ptr == rhs.ptr);
    }

bool operator!=(this_t const &rhs) const
    {
    return !operator==(rhs);
    }
bool is_valid_ptr() const
    {
    return (ptr >= container.mem_start
        && ptr < container.seq_end);
    }

bool is_valid_ptr2() const
    {
    return (ptr >= container.mem_start
        && ptr <= container.seq_end);
    }

Context

StackExchange Code Review Q#94211, answer score: 6

Revisions (0)

No revisions yet.