Return to Answer

Questions:

1. What is the slight deviation from the standard interface in the explicit Vector(size_type capacity)? Is it that the param is named 'capacity' and not 'count?

If you look at the standard (I link to a non standard but reputable source) std::vector::vector you will see there is no constructor that takes a "capacity" (ie. you have allocated space but zero size). There is one that takes a size and fills it with values (but you have that one).

2. When you say "T()" should be in the header only, do you mean the template declaration?

No. Default Parameters should be defined in the class header file only. They are not part of the declaration:

class Vector
{
    void resize(size_type count, const_reference value = T());
    // This is good.
    // Reading the interface you see you can have a default value.
};


// Don't define the default parameters here.
// It should generate an error if you do this.
void Vector::resize(size_type count, const_reference value) 
{
    // STUFF
} 

3. How does constructing the item in place in push_back() resolve the uninitialized memory issue?

This is a problem becasue:

  // This code uses the assignment operator.
  data[m_size++] = item;

  // The assignment operator assumes that the object
  // referenced on the left hand side has already been constructed
  // but in your case that is not true, this is unintialized memory.
  // So you are using assignment to uninitialized memory
  // which could be anything and thus with non trivial T
  // will cause an issue.

This is solved by constructing in place:

        // Should be this.
        new (std::addressof(data[m_size++])) T(item);

The constructor assumes the memory has not been constructed before. You pass the address to operator new it will not allocate space but simply use the pointer provided as the location and then call the constructor for the type T to correctly initialize the memory.

4. do you have any references or documentation on the "pass by value" idiom for avoiding the two assignment operators?

Nope. There are lots of questions on Stackoverflow that go over this though. Should be simple to find.

5. And does the SFINAE approach involve checking std::is_nothrow_move_constructible or something in overload resolution? I should look into how this is done in some compiler implementation

Yep.

But what about const iterators or reverse iterators or const reverse iterators?

    iterator               begin()   const {
    const_iterator         cbegin()  const {
    reverse_iterator       rbegin()  const {
    const_reverse_iterator crbegin() const {

But pointed out by @chrysante below, the std::vector default constructor is noexcept so can't have any memory allocation (as that can potentially throw). So if you want to go that route you can mark this default constructor noexcept.

    Vector()  noexcept
        : data{nullptr}
        , m_size{0}
        , m_capacity{0}
    {}

One comment on style. Like in variable declarations in the code blocks its nice to have member initialization in the constructor one per line (its easier to read). You are not trying to save vertical space.

But what about const iterators or reverse iterators or const reverse iterators? Also when calling begin() on a const object you will get a const_iterator.

    iterator               begin()         { // normal begin
    const_iterator         begin()   const { // begin on const object
    const_iterator         cbegin()  const { // explicitly asking for const
    reverse_iterator       rbegin()        { // normal rbegin
    const_reverse_iterator rbegin()  const { // rbegin on const object
    const_reverse_iterator crbegin() const { // explicitly asking for re

As vector is such a common thing we review here I have written up a series of articles about implementing a vector:

Found one major bug in push_back(). You have potential memory leaks in reserve() and shrinktofit() easy to fix. You can simplify your assignment operator (currently you have copy and move versions) they can be combined into a single version that works for both.

Codereview

    // Notice the parameter is value.
    // If passed a RValue it is moved into the parameter.
    // If passed an LValue it is copied.
    // So you get the same affect with less code.
    Vector& operator=(Vector other) noexcept
    {
        swap(other);
        return *this;
    }

But what about accesses to a const Vector. Just because you can't modify does not mean you can't use the operator[] on it.

This is fine. But std::vector destroys them from back to front. Just like how it deletes elements during destruction. This is to mimik the behavior of the C-Array (objects are destroyed in reverse order of creation).

The one thing here I would watch is that you have a potential leak:

It's hard to spot. But if the type T does not support move construction then the compiler will use the copy constructor during the std::uninitialized_move. If one of the copy constructors fails (i.e. throws) then you will leave thus function with cleaning up newData. Though your function does provide the strong exception guarantee.

    // Note: Pass by value.
    //       RVALUE are them moved into item then moved to container.
    //       LVALUE are copied into item then moved to the container.
    void push_back(T item) {
        reserve_if_needed();
        new (std::addressof(data[m_size++])) T(std::move(item));
    }

        if (m_size > 0) {
            // Why not swap the next two lines.
            // This would make the line to call the destructor
            // simpler and easier to read as you don't need the -1
            data[m_size - 1].~T();
            m_size -= 1;
        }

To be similar to C-Array (and the C++ standard idiom that objects are destroyed in reverse order of creation). You should destroy the members in reverse order.

No need to check for nullptr!

As vector is such a common thing we review here I have written up a series of articles about implementing a vector.

I found one major bug in push_back(). You have potential memory leaks in reserve() and shrinktofit() that are easy to fix. You can simplify your assignment operator (currently you have copy and move versions) they can be combined into a single version that works for both.

Code review

    // Notice the parameter is value.
    // If passed a RValue it is moved into the parameter.
    // If passed an LValue it is copied.
    // So you get the same effect with less code.
    Vector& operator=(Vector other) noexcept
    {
        swap(other);
        return *this;
    }

But what about accesses to a const Vector? Just because you can't modify does not mean you can't use the operator[] on it.

This is fine. But std::vector destroys them from back to front. Just like how it deletes elements during destruction. This is to mimic the behavior of the C-style array (objects are destroyed in reverse order of creation).

The one thing here I would watch is that you have a potential leak.

It's hard to spot. But if the type T does not support move construction then the compiler will use the copy constructor during the std::uninitialized_move. If one of the copy constructors fails (i.e. throws) then you will leave this function needing to clean up newData. Though your function does provide the strong exception guarantee.

    // Note: Pass by value.
    //       RVALUE are moved into item then moved to container.
    //       LVALUE are copied into item then moved to the container.
    void push_back(T item) {
        reserve_if_needed();
        new (std::addressof(data[m_size++])) T(std::move(item));
    }

        if (m_size > 0) {
            // Why not swap the next two lines?
            // This would make the line to call the destructor
            // simpler and easier to read as you don't need the -1
            data[m_size - 1].~T();
            m_size -= 1;
        }

To be similar to C-style arrays (and the C++ standard idiom that objects are destroyed in reverse order of creation), you should destroy the members in reverse order.

No need to check for null pointers here!