Vector class reimplementation as dynamic array: Memory faults with copy constructor

Question

For a programming lecture I'm trying to reimplement some of C++ standard vector class' functionality in a own vector class. I've looked at several similar classes and implementations now and my implementation is (save for using initializer lists) quite the same and most functions are working as intended so far. I've kept this example intentionally simple to read, to at least lessen the chance for syntax errors.

However, my copy constructor and I suppose my push_back function are causing memory leaks and I can't pinpoint why, even after rewriting the code for them several times and comparing them to similar implementations.

Here is my vector.h:

#include<iostream>
#include <initializer_list>
using namespace std; 

class Vector {

private:
size_t sz;
size_t max_sz;
double* values;
static constexpr size_t min_sz {5};

public:

    //default constructor
    Vector()
    {
        values = new double[min_sz];
        sz = 0;
        max_sz = 5;
    }

    //constructor with submitted variable

    Vector(size_t n)
    {
        if (n < min_sz)
        {
            n = min_sz;
        }
        values = new double[n];
        sz = 0; 
        max_sz = n;

    }

    //copy constructor
    Vector (const Vector &v)
    {   
        max_sz=v.max_sz;
        sz=v.sz;
        values = new double[max_sz];

        for(size_t i=0;i<v.sz;i++)
        {
            values[i]=v.values[i];

        }

       /*
        cout << "[Copy constructor called with params Max_sz:" << max_sz << " and sz: " << sz;
        cout << "\n referenced vector contains: ";
        cout << v;
        cout << " | ";
        cout << "this vector contains: " << *this;
       */

    }

    Vector (std::initializer_list<double> list)
    {


        if(list.size() == 3)
        {
            std::initializer_list<double>::iterator it = list.begin();
            sz = *it++;
            max_sz = *it++;
            if (max_sz < min_sz)
            {
                max_sz = 5;
            }
            values = new double[max_sz];
            values[0] = *it;

        }



    }


    //destructor
    ~Vector()
    {
        delete[] values;
    }

    //functions

    size_t size() const
    {
        return sz;
    }

    bool empty() const
    {
        if (sz < 1) { return true; }

        return false;
    }


    void clear()
    {
        //set number of elements to 0, reserved memory isn't touched but can be overwritten by new assignments now

    sz = 0;

    }

    void reserve(size_t n)
    {
        if (n > max_sz)
        {
            double* tmp = new double[n];

            for (size_t i = 0; i < n; i++)
            {
                tmp[i] = values[i];
            }  

        delete [] values;
        values = tmp;

        }


    }


    void shrink_to_fit()
    {
        if (sz != max_sz)
        {
            double* tmp = new double[sz];

            for (size_t i = 0; i < sz; i++)
            {
                tmp[i] = values[i];
            }

            delete [] values;
            values = tmp;
        }
    } 

    void push_back(double x)
    {
        //check for space, if not enough space is allocated, allocate more

        if (sz+1 >= max_sz)
        {
            double* tmp = new double[max_sz*2];

            for (size_t i = 0; i < sz; i++)
            {
                tmp[i] = values[i];
            }

        delete [] values;
        values = tmp;
        max_sz = max_sz*2;

        }

        //then or otherwise, add the new element

        values[sz] = x;
        sz++;



    }

    void pop_back()
    {
        //remove last element in vector, throw exception if vector is empty

        if (sz == 0)
        {


        }
        else
        {
            //create new vector in memory with space for one element less

            sz--;

            double* tmp = new double[sz];

            for (size_t i = 0; i < sz; i++)
            {
                tmp[i] = values[i];
            }

            delete [] values;
            values = tmp;

        }




    }



    double& operator[](size_t index)
    {
        return values[index];
    }

    const double& operator[](size_t index) const
    {
        return values[index];
    }

    size_t capacity() const
    {
        return max_sz;
    }

    friend ostream& operator<< (ostream& out, const Vector& v)
    {
        out << "[";
        for (size_t i = 0; i < v.sz; i++)
        {
            out << v.values[i];
            if (i+1 < v.sz)
            {
                out << ",";
            }

        }

        out << "]";

        return out; 
    }


};

Here is a test case for causing the memory fault:

int main(){

  Vector y;
  cout << y << endl;
  {
    Vector x(0);
    for(double i{0};i<1;i+=0.1)
      x.push_back(i);
    cout << x << endl;
    x.pop_back();
    cout << x << endl;
    x.shrink_to_fit();
    cout << x << endl;
    x.push_back(10);
    const Vector tmp{x};
    const Vector tmp2{tmp};
    y = tmp2;
  }
  cout << y << endl;
  cerr << y << endl;
  Vector x{1,2,3};
  for(size_t i{4};i<10000;++i)
    x.push_back(i);
  cout << "Done" << endl;

  return 0;
}

Here is a link to the compiler I'm testing it with, with the class and test case already saved:

https://rextester.com/EJFO89500

Can someone give me a hint on where the flaw in my code is?

Answer 1

One issue is this line:

y = tmp2;

That line uses the assignment operator, which you failed to implement. What winds up happening is that the default assignment operator is invoked, which only does a shallow copy. Thus you have values duplicated in two objects, and the destructor will have issues when issuing a delete[] on the same value twice.

The easiest way to implement an assignment operator is to use the copy / swap idiom:

#include <algorithm>
//...
class Vector {
   public:
      Vector& operator=(const Vector& rhs)
      {
         if ( this != &rhs )
         {
            Vector temp(rhs);
            std::swap(temp.sz, sz);
            std::swap(temp.max_sz, max_sz);
            std::swap(temp.values, values);
        }
        return *this;
     }
//...
};

Basically, we create a temporary of the passed-in Vector, and exchange the current contents with the temporary's contents. Then the temporary dies off with the old contents.