user1888527
user1888527

Reputation: 565

Why is this giving me an access violation? (C++)

I'm building a vector class for my data structures class, and I can't figure out why this is throwing an exception. Here's the complete Vector.h file:

#include <iostream>

using namespace std;

template <class T>
class Vector {
private:

  // not yet implemented
  Vector(const Vector& v);
  Vector& operator=(const Vector& v);
  T * Tarray;
  int arraySize;
  int currentSize;

public:

Vector() {
    arraySize = 2;
    currentSize = 0;
    Tarray = new T[arraySize];
};
~Vector() {
    delete[] Tarray;
};

void push_back(const T &e) {
    ++currentSize;
    if (currentSize > arraySize) {
        arraySize *= 4;
        T * temp = new T[arraySize];

        for (int i = 0; i < currentSize; i++) {
            temp[i] = Tarray[i];
        }

        delete[] Tarray;
        Tarray = new T[arraySize];

        for (int j = 0; j < currentSize; j++) {
            Tarray[j] = temp[j];
        }

        delete[] temp;

        Tarray[currentSize - 1] = e;
    }

    else {
        Tarray[currentSize - 1] = e;
    }
};

void print() {
    for (int i = 0; i < currentSize; i++) {
        cout << Tarray[i] << "  ";
    }

};

int getCurrentSize() {
    return currentSize;
};

int getArraySize() {
    return arraySize;
};

// Not yet implemented
void pop_back();

int size() const;

T& operator[](int n);


};

And here's my complete main.cpp I was using to test it.

#include "Vector.h"
#include <iostream>
#include <string>

using namespace std;

int main() {
char c;

string * temp = new string[8];


Vector<string> newVector;

for (int i = 0; i < 8; i++) {
    newVector.push_back("Hello world");
    newVector.push_back("Hello world");
}

newVector.print();
cout << endl << "Current Size: " << newVector.getCurrentSize();
cout << endl << "Array Size: " << newVector.getArraySize();
cin >> c;
}

Upvotes: 0

Views: 131

Answers (1)

Mats Petersson
Mats Petersson

Reputation: 129364

I would rewrite push_back as follows:

void push_back(const T &e) {
    if (currentSize+1 > arraySize) {
        arraySize *= 4;
        T * temp = new T[arraySize];

        for (int i = 0; i < currentSize; i++) {
            temp[i] = Tarray[i];
        }

        delete[] Tarray;
        Tarray = temp;
    }
    Tarray[currentSize] = e;
    ++currentSize;
};

Changes are:

  • Don't update currentSize until after you have copied the contents (thus not going out of bounds in Tarray).
  • Don't allocate and copy twice. Just assign Tarray to temp after deleting it.
  • Only stick element into Tarray in one place.
  • Update currentSize after, to avoid having to do -1 (It does require a single +1 in the first if instead.

Upvotes: 5

Related Questions