CODEWITHSUNDEEP
c++memorydynamicvectorimplementation
Stanimirovv
Stanimirovv

Reputation: 3172

Error with the Copy constructor and equality operator

I am making my own implementation of the class "vector" with templates as practise. Everything seems to be working except the Copy constructor and the overloaded = operator. I am not sure where exactly the problem is. I will post the entire source code below:(Also I am not sure if I have any leaks)

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

template <typename TT1>
class Vector{
private:
    int size;
    TT1* ptr;
public:
    Vector();
    ~Vector();
    Vector(int size);
    Vector(const Vector& a);
    Vector<TT1>& operator=(const Vector& a);
    void set(int position, TT1 data);
    TT1 get(int position);
    int get_size();
};

//------Default Constructor------
template <typename TT1>
Vector<TT1> :: Vector()
{
    size = 0;
    ptr = NULL;
}

//-------Copy Constructor-------
template <typename TT1>
Vector<TT1> :: Vector(const Vector<TT1>& a)
{
    if(a.ptr != NULL)
    {
        this->size = a.size;
        delete[] ptr;
        ptr = new TT1[size];
        for ( int i = 0; i < this->size; i++)
                this->ptr[i] = a.ptr[i];
    }
}

//------Destructor-------
template <typename TT1>
Vector<TT1> :: ~Vector()
{
    delete[] ptr;
}

//-----Overloaded = operator----
template <typename TT1>
Vector<TT1>& Vector<TT1> :: operator=(const Vector<TT1>& a)
{
    if(this != &a)
    {
        this->size = a.size;
        delete[] this->ptr;
        this->ptr = new TT1[a.size];
        for (unsigned int i = 0; i < this->size; i++)
                this->ptr[i] = a.ptr[i];
        return *this;
    }
}
//----Constructor with size---
template<typename TT1>
Vector<TT1> :: Vector(int size)
{
    this->size = size;
    ptr = new TT1[size];
}

//---- Set data @ Poisition----
template<typename TT1>
void Vector<TT1> :: set(int position, TT1 data)
{
    *(ptr+ position) = data;
}

//----Get data From position----
template<typename TT1>
TT1 Vector<TT1> :: get(int position)
{
    return *(ptr + position);
}

//-----Get size----
template<typename TT1>
int Vector<TT1> :: get_size()
{
    return size;
}

void foo(Vector<string> a)
{

}

int main()
{
    Vector<string> a(3);
    a.set(0, "asd");
    a.set(2, "hjk");
    a.set(1, "34645!");
    for(int i = 0; i < a.get_size(); i++)
    {
        cout << a.get(i) << endl;
    }
    Vector<string> b = a;
    for(int i = 0; i < a.get_size(); i++)
    {
        cout << b.get(i) << endl;
    }
    foo(a);

    return 0;
}

And now the parts where the mistakes are:

template <typename TT1>
Vector<TT1> :: Vector(const Vector<TT1>& a)
{
    if(a.ptr != NULL)
    {
        this->size = a.size;
        delete[] ptr;
        ptr = new TT1[size];
        for ( int i = 0; i < this->size; i++)
                this->ptr[i] = a.ptr[i];
    }
}

And finally here:

//-----Overloaded = operator----
template <typename TT1>
Vector<TT1>& Vector<TT1> :: operator=(const Vector<TT1>& a)
{
    if(this != &a)
    {
        this->size = a.size;
        delete[] this->ptr;
        this->ptr = new TT1[a.size];
        for (unsigned int i = 0; i < this->size; i++)
                this->ptr[i] = a.ptr[i];
        return *this;
    }

Upvotes: 1

Views: 70

Answers (1)

Matt Phillips
Matt Phillips

Reputation: 9691

You delete a pointer that was never allocated:

//-------Copy Constructor-------
template <typename TT1>
Vector<TT1> :: Vector(const Vector<TT1>& a)
{
    std::cout << "Vector(const Vector<TT1>& a), " << a.size << std::endl;
    if(a.ptr != NULL)
    {
        this->size = a.size;
        delete[] ptr;  //<-------------- no corresponding new
        ptr = new TT1[size];
        for ( int i = 0; i < this->size; i++)
                this->ptr[i] = a.ptr[i];
    }
}

Just comment that out, and it works fine.

Edit: Per Mike Seymour's observation, the members esp. ptr should be initialized:

Vector<TT1> :: Vector(const Vector<TT1>& a) : size(0), ptr(NULL)
{
...
}

In addition, although the compiler accepts it for constructor/operator= apparently, you will otherwise get an 'incomplete type' error if you write code of the form

Vector(const Vector& a);
Vector<TT1>& operator=(const Vector& a);

Rather, for consistency and good style imho include the template parameters:

Vector(const Vector<TT1>& a);
Vector<TT1>& operator=(const Vector<TT1>& a);

And similarly in the definitions.

Upvotes: 1

Related Questions