0

PART OF MY VECTOR CLASS

class CVector
{
    friend ostream &operator<<(ostream& output, const CVector& vec);

public:

    CVector()
    {
        sPtr = new char[10];
        size = 0;
        capacity = 10;
    }

    CVector(const CVector &vec)
    {
        size = vec.size;
        capacity = vec.capacity;
        sPtr = new char[size];
        cout << "copy constructor" << endl;
        for (size_t i = 0; i < size; i++)
        {
            sPtr[i] = vec.sPtr[i];   
        }
        
    }

Main Function

CVector veca;
    cout << veca.getCapacity() << endl;
    cout << veca.getSize() << endl;
    veca.push_back('H');
    veca.push_back('e');
    veca.push_back('l');
    veca.push_back('l');
    veca.push_back('o');
    cout << "Add five elements:Hello" << endl << "result:" << veca << " size:" << veca.getSize() << " capacity:" << veca.getCapacity()<<endl;
    CVector vecb,vecc;
    vecb.push_back('H');
    vecb.push_back('i');
    vecc = vecb;

After this code(vecc = vecb),the issue happend.I think something went wrong >with copyconstructor but i cannot figure out what's wrong

push_back function

void push_back(char c)
    {
        if (size == capacity-1)
        {
            char* temp = new char[2 * capacity];
            for (size_t i = 0; i < capacity; i++)
            {
                temp[i] = sPtr[i];
            }
            delete[] sPtr;
            capacity *= 2;
            sPtr = temp;
            
        }
        sPtr[size] = c;
        size++;
     
    }

Can I assign a *temp local pointer to the *sPtr(pointer to array for my vector)

Toqgcm
  • 33
  • 5
  • No copy constructor is used in the code provided. – Eljay Apr 10 '21 at 16:22
  • Please provide the full definition of push_back, the copy assignment operator and the destructor. As @Eljay mentioned the statement `vecc = vecb;` will call the copy assignment operator and not the copy constructor. – benb Apr 10 '21 at 19:13
  • If there is no copy assignment operator (but a destructor), this will fail, because the internal array is copied by reference. – PMF Apr 10 '21 at 20:14
  • Thank you for the replies, now I know copy constructor is using for ```Cvecc(vecb)``` – Toqgcm Apr 11 '21 at 03:06
  • @benb I have put the push_back funtion above – Toqgcm Apr 11 '21 at 04:31
  • This assertion indicates that the pointer that should be released is invalid (or no longer valid). I suggest you could refer to the link: https://stackoverflow.com/a/64418625/11872808 You allocated memory in your constructor and so you need to write a destructor to delete it. Otherwise, you will cause a memory leak. if a copy is made of your object, then the copy will point to the same memory as the original object. Once, one of these deletes the memory in its destructor, the other will have a pointer to invalid memory. – Jeaninez - MSFT Apr 13 '21 at 09:21

1 Answers1

0

In your copy constructor, you allocate vec.size bytes, but set capacity to vec.capacity. You probably want to allocate vec.capacity bytes instead.

The next thing is, that there is a difference between copy construction and copy assignment:

CVector vecb(veca); // this calls the copy constructor
CVector vecc; // this calls the default constructor
vecc = vecb; // this calls the copy assignment operator

Since you haven't given an implementation for the copy assignment operator, the compiler will generate one for you, which looks something like that:

CVector & operator = (const CVector & other) {
    size = other.size;
    capacity = other.capacity;
    sPtr = other.sPtr;
}

This is not suitable for you, because

  1. you lose track of sPtr and nobody deletes it.
  2. two different CVectors now point to the same memory via sPtr. When the destructor of the first CVector is called, it will delete sPtr. Then the other CVector will point to memory that is already deleted. When it's destructor (or any other member function) is called, it tries to delete or access that deleted memory.

That is the reason for the rule of 3 (or rule of 5, depending on whether you implement move semantics).

An easy and good way to implement the copy (and move) assignment operator, is to use the copy and swap idiom:

void swap(CVector & other) {
    std::swap(size, other.size);
    std::swap(capacity, other.capacity);
    std::swap(sPtr, other.sPtr);
}

CVector & operator = (const CVector& other) {
    CVector tmp(other);       // make a copy of other
    this->swap(other);        // by swapping with other, tmp's destructor, will cleanup your previous sPtr
    return *this;
}
Dharman
  • 30,962
  • 25
  • 85
  • 135
benb
  • 174
  • 2
  • 4