Copy constructor producing incorrect copy of old array

Question

I have a class vector210, in which I am trying to create a copy constructor as is outlined below in the sample code below (only part of the complete code).

class vector210 {
    public:
    int arraySize;
    int values[1]; 

vector210(int array[], int arraySize_){
arraySize = arraySize_;
for(int i = 0;i !=arraySize_;i++){
    values[i] = array[i];
    }
}

vector210(const vector210 &p){
    int values [p.arraySize];

    for(int i=0;i<p.arraySize;i++){
    values[i] =  p.values[i];            
          };        
    arraySize = p.arraySize;

};


void print(){
for(int i =0;i <arraySize;i++){
    cout << values[i]  << endl;
    }

if (arraySize ==0){
    cout << "Vector is empty." << endl;
}
};

when I run the code in main as:

#include "CST210vector.h"
#include <iostream>
using namespace std;
int main() {
    int v[5] = {1,2,3,4,5} ;
    vector210 test(v, sizeof(v)/sizeof(*v));

    cout << "Array to copy " << endl;
    test.print();
    cout << "Copied array values:"<< endl;
    vector210 testnew = test;
    testnew.print();
    cout << " " << endl;
    cout << testnew.size() << endl; 
}

I receive the output to terminal:

Array to copy
1
2
3
4
5
Copied array values:
5
4198189
0
1
2

So somehow it seems like the array that is constructed when the copy constructor is called is wildly different from the array in the old version of the vector210 object, but I am not sure how this is happening. Does anyone have insight as to how this error is coming about? I wish instead for my copy constructor to produce an exact copy of the original array.

Why values[1]? Is your vector supposed to hold only one element? — el.pescado - нет войне
– el.pescado - нет войне, Commented Mar 6, 2018 at 6:41
You have an array of 1 element. What possessed you to think you can assign an arbitrary amount of elements to it? — StoryTeller - Unslander Monica
– StoryTeller - Unslander Monica, Commented Mar 6, 2018 at 6:42
In your copy constructor, int values [p.arraySize]; does not resize values member variable, it declared new local variable that shadows member variable. — el.pescado - нет войне
– el.pescado - нет войне, Commented Mar 6, 2018 at 6:44
If you are just starting out, use std::vector instead of re-inventing it. If you are doing it as an exercise, and you haven't covered dynamic memory allocation yet, you aren't ready to implement it. — StoryTeller - Unslander Monica
– StoryTeller - Unslander Monica, Commented Mar 6, 2018 at 6:46

john · Accepted Answer · 2018-03-06 07:02:57Z

1

Your approach is wrong vector210::values should be a pointer and you should allocate memory for it. C++ doesn't have dynamic arrays. Your class and constructor should look like this

class vector210 {
public:
    int arraySize;
    int* values; // pointer!

    vector210(const int* array, int arraySize_) : 
            arraySize(arraySize_),
            values(new int[arraySize_]) { // allocate memory
        for (int i = 0; i != arraySize_; i++)
            values[i] = array[i];
    }
    ...
}

Now your copy constructor should be similar, it allocates memory using new.

answered Mar 6, 2018 at 7:02

john

88.3k5 gold badges61 silver badges95 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Ch1v1 · Accepted Answer · 2018-03-06 10:07:35Z

0

You are using an dinamic array. Compiler should complain about it

I will suggest use a pointer as int a [] become int * a, the following works fine to me

class vector210 {
    public:
    int arraySize;
    int *values;

vector210(const vector210 &p){
    values = new int[p.arraySize];
    memcpy ( values , p.values, p.arraySize*sizeof(int) );
    p.values[1] = 0;
    arraySize = p.arraySize;
}

edited Mar 6, 2018 at 10:07

answered Mar 6, 2018 at 9:31

Ch1v1

1211 silver badge7 bronze badges

Collectives™ on Stack Overflow

Copy constructor producing incorrect copy of old array

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related