C++ Assignment Operator - Vector Substrict out of range

[venros]

Hi guys,

My assignment operator is supposed to copy over the referenced vector into a new array and update exsisting vector.

So:

Lawtest (has a vector size of 50 and initialized 0-49.
mtest takes all the values of Lawtest.vector and copies it into mtest.vector and mtest.arrayofint[]

However it is only copying 32 indexes and not all 50 into the new array; and gives me the error:

Vector Substrict Out of Range

here is the relevant code:

class vectorOfInt {
private:
  std::vector<int> vectorInt;

  int *arrayIntPtr; 

  static const int CAPACITY = 32;

Public:

// • A constructor that takes an initial size as the argument
inline vectorOfInt::vectorOfInt(int s){
  std::cout << "Invoking an initial size argument constructor. " << std::endl;
  vectorInt.reserve(s); // < resize
  std::cout << "The new size/capacity is: " << vectorInt.capacity() << std::endl;
  for(std::size_t i = 0; i != vectorInt.capacity(); i++){
    vectorInt.push_back(i+1);
  }
}

// Assignment operator
inline vectorOfInt& vectorOfInt::operator= (const vectorOfInt & other){
  std::cout << "Assignment Operator Initialized. " << endl;

  if(this == &other){
    return *this;
  }

  arrayIntPtr = new int [other.vectorInt.capacity()];

  copy(other.vectorInt.begin(), other.vectorInt.end(), this->arrayIntPtr);

  //clear old vector and copy it.
  this->vectorInt.clear();
  this->vectorInt.reserve(other.vectorInt.capacity());

  for(std::size_t i = 0; i != other.vectorInt.capacity(); i++){
    this->vectorInt[i] = other.vectorInt[i];
  }
}
}

and here is my main:

  vectorOfInt Lawtest(50), mtest;
  mtest = Lawtest;

    cout << "Printing mtest array. " << endl;
    mtest.printArrayData();
    cout << "Printing mtest vector: " << endl;
    mtest.printVectorData();

ty

Forgot to add the no argument constructor for mtest:

// • A no-argument constructor that allocates a 32 element vector
inline vectorOfInt::vectorOfInt(){
  std::cout << "Invoking a no argument constructor. " << std::endl;
  vectorInt.reserve(CAPACITY);
  for(std::size_t i = 0; i != vectorInt.capacity(); i++){
    vectorInt.push_back(i);
  }
}

[keskiverto]

mtest.vectorInt starts empty.
Then you clear it. Still empty.
You reserve some space. Still empty.
Therefore, there are no elements to access with [].

[venros]

but lawtest is not empty, and the operator does infact copy 32 elements from lawtest.vector to mtest.array.

But as you can see lawtest has 50 elements not 32, and it fails to copy those remaining 18 elements to the array.

It also doesnt copy a single thing to mtest.vector.

  for(std::size_t i = 0; i != vectorInt.capacity(); i++){
    vectorInt.push_back(i+1);
  }
}

The code above allocates lawtest

[Lachlan Easton]

// mtest.vectorInt starts empty
// Then you clear it. Still empty.
this->vectorInt.clear();
// You reserve some space. Still empty.
this->vectorInt.reserve(other.vectorInt.capacity());

for(std::size_t i = 0; i != other.vectorInt.capacity(); i++){
    // Therefore, there are no elements to access with [].
    this->vectorInt[i] = other.vectorInt[i];
    ^^^^^^^^^^^^^^^^^^
}

[venros]

I updated the code to show that mtest.vector is infact not empty and has 32 elements.

Also :

    // Therefore, there are no elements to access with [].
    mtest.vectorInt[i] = Lawtest.vectorInt[i];

The other.vector ( lawtest.vector) is allocated for all 50 elements and that is what I am copying from.

I am clearning this-> vector which is mtest to clear all 32 elements and replace them with all 50 elements from lawtest. This is my issue.

[norm b]

A suggestion: Try running it with vector::clear() commented out //this->vectorInt.clear()

From here: http://www.cplusplus.com/reference/vector/vector/clear/

Removes all elements from the vector (which are destroyed), leaving the container with a size of 0.

[firedraco]

The actual problem here is:

for(std::size_t i = 0; i != other.vectorInt.capacity(); i++){

Why are you looping to other.vectorInt.capacity()? The vector might not be filled. Use .size() instead.

[venros]

norm b, that didnt compile for me.

firedraco, Im still getting (after using size):

Expression: Vector subscript out of range

Also didnt know it still mattered as long as I reserved the space.

The issue is def in the following code as I commented it out and it compiled:

 for(std::size_t i = 0; i != other.vectorInt.size(); i++){
    this->vectorInt[i] = other.vectorInt[i];
  }

[keskiverto]

Also didnt know it still mattered as long as I reserved the space.

It does. Size is not capacity. The reserve() and resize() are different. Reread the vector's documentation.

Now that you do show the default constructor, we can see that the mtest is not empty like we first assumed; it has size 32, initially.

[venros]

anyone?

[norm b]

You have an empty vector (mtest) because you called clear() and you want to add the elements from Lawtest to it. If you want to add elements to a vector what function do you use?

[cire]

To make what norm b said a little more clear, in the following code:

  //clear old vector and copy it.
  this->vectorInt.clear();
  this->vectorInt.reserve(other.vectorInt.capacity());

  for(std::size_t i = 0; i != other.vectorInt.capacity(); i++){
    this->vectorInt[i] = other.vectorInt[i];
  }

On line 35, you invoke the clear method of vectorInt. clear sets the size of the vector to 0. As has already been noted, the capacity and size of the vector are different things, so when you set the capacity on line 36, you do not affect the size of the vector at all.

When you reach line 39, vectorInt has no elements. In other words, there is no valid index which may be used with vectorInt. Use resize on line 36 (and resize to the size of other.vectorInt, not the capacity.) The use of capacity on line 38 has already been addressed. The clear on line 35 is unnecessary since you're going to overwrite those elements anyway.

By the way, whatever your arrayIntPtr is intended to do, it's not doing it. You don't treat it consistently and it is entirely unsafe to use most of the time, given the constructors you've supplied (and two consecutive uses of operator= on the same object will result in a memory leak.)

[venros]

Tyvm guys! it finally worked,

@cire, this code is not supposed to be optimal or prevent memory leaks, its a crappy code i wrote just to practice assignment operators and copy constructors. I just needed something quick to mess with, and something to give me the most amount of errors so I can get a deeper understanding.

I appreciate all your help guys.

You can mark this answered.