Dynarray implementation with iterator

So here is an implementation of a dynarray. Many people might not know about it, so here is a reference.

Note that I am not trying to write a 100% standard conforming implementation.
Also, this is for a library, so resist the urge to scold me on using a leading underscore for two of my variables.

```
#include
#include
#include
#include

template
class dynarray
{
public: //TYPE ALIASES
using value_type = T;
using size_type = std::size_t;
using difference_type = std::ptrdiff_t;
using reference = T&;
using const_reference = const T&;
using pointer = T*;
using const_pointer = const T*;

private: //ITERATOR
template
class dynarray_iter : public std::iterator
{
private:
U* pos = nullptr;

public:
constexpr explicit dynarray_iter(U* position)
: pos(position)
{}

constexpr dynarray_iter() = default;
constexpr dynarray_iter(const dynarray_iter&) = default;
dynarray_iter& operator=(const dynarray_iter&) = default;

//INCREMENT/DECREMENT OPERATORS
dynarray_iter& operator++() { ++pos; return *this; }
dynarray_iter& operator--() { --pos; return *this; }
dynarray_iter operator++(int) { dynarray_iter temp(*this); operator++(); return temp; }
dynarray_iter operator--(int) { dynarray_iter temp(*this); operator--(); return temp; }

//ARITHMETIC OPERATORS
dynarray_iter operator+(difference_type off) const { return dynarray_iter(pos + off); }
dynarray_iter operator-(difference_type off) const { return dynarray_iter(pos - off); }

dynarray_iter& operator+=(difference_type off) { pos += off; return *this; }
dynarray_iter& operator-=(difference_type off) { pos -= off; return *this; }

difference_type operator-(const dynarray_iter& rhs) const { return pos - rhs.pos; }

friend dynarray_iter operator+(difference_type off, const dynarray_iter& it) { retur

Include the appropriate headers

You need to include ` if you're going to be using std::swap. Now that we have that header included, we can use it to reduce the size of some of the functions you've implemented.

Use the standard when you can

Many of your functions can be simplified now that we've included . For example, your fill function.

void fill(const T& val)
{
    std::fill(_data, _data + _size, val);
}

I would also remove that unnecessary //FILL comment you have as it is clear from the function name what you're doing.

Your "fill constructor" can also be simplified:

dynarray(std::size_t count, const T& fill_val)
{
    _data = new T[count];
    fill(fill_val);
    _size = count;
}

I also removed the default-value from the constructor as I think that if we're using a fill constructor, we should be providing it with a value to fill it with.

Your "iterator constructor" can be simplified to:

template
dynarray(InputIt first, InputIt last)
{
    _size = std::distance(first, last);
    _data = new T[_size];
    std::copy(first, last, _data_);
}

Copy constructor:

dynarray(const dynarray& rhs)
{
    _data = new T[rhs.size()];
    std::copy(rhs.begin(), rhs.end(), _data);
    _size = rhs.size();
}

Wrong interface

According to the link you provided, a dynarray is:


neither copy-, nor move-assignable

Therefore, you should be delete-ing your entire copy, move, and assignment operator functions. Technically, your implementation isn't correct but you've addressed that in your post.

Memory management
As it stands, your class leaks memory each time it is copied. In your copy c-tor, you never delete the memory you're holding on to before allocating a new piece. I would look into using some smart pointers to avoid having to worry about this issue(std::unique_ptr). If you do switch to a smart pointer allocator, then you don't even need to provide a destructor as cleanup is done for you by the smart pointer. Example:

// assume _data is of type std::unique_ptr
dynarray(std::size_t count, const T& fill_val)
{
    _data = std::make_unique(count);
    fill(fill_val);
    _size = count;
}

Then in your copy c-tor, you can do this:

dynarray(const dynarray& rhs)
{
    _data = std::make_unique(rhs.size()); // no leak
    std::copy(rhs.begin(), rhs.end(), _data.get());
    _size = rhs.size();
}

And your swap function can be:

void swap(dynarray& rhs) noexcept
{
    _data.swap(rhs._data);
    std::swap(_size, rhs._size);
}

You'll have to adjust the rest of your interface to take into account your new unique_ptr implementation but your implementation is now leak-free.

You can simply this logic:

T& at(std::size_t i)
{
    if (!(i < _size)) {
        throw std::out_of_range("Index out of bounds!");
    }
    return _data[i];
}

to:

T& at(std::size_t i)
{
    if (i >= _size) {
        throw std::out_of_range("Index out of bounds!");
    }
    return _data[i];
}

For this function signature:

std::size_t size() const noexcept { return _size; }

you have size_type` defined as std::size_t. Use it.

size_type size() const noexcept { return _size; }

StackExchange Code Review Q#116085, answer score: 5