C++: How to properly copy objects stored in a dynamic-array in a class?

Question

It's a long time since I "learned" C++, and I haven't really used it for some years. I need help making a copy-constructor -- possibly a constructor -- and perhaps overloading the =-operator. I suddenly can't seem to make this correct...

One thing, objects (created from a class) and referred by name (e.g. sys1), is the name then a pointer (like an array) or an actual instance (like an int)?

I have one class (Sys) that contains a dynamic-array of instances of another class (Rad) (or, it's supposed to). I want to copy the content of one instance of the Sys-class to another (possibly empty) instance of the same class... and I'm obviously doing something wrong.

For one attempt, I was "double-deleting", so I'd obviously made a shallow copy. For the other attempt, I failed to copy over the content of the Rad-instances in the dynamic array.

So how should this be done *correctly*? Hos should the copy-constructor be? How should the constructor be? Do I need to overload =-operator? Should I use pointer and new? How should the assignment be done (e.g. "s3=s1", "s3=Sys(s1)", or something else)?

I have the following two classes:

class Rad {
private:
int data[11];
public:
//Nothing interesting, using default constr, destr and cpyconstr
};

//The methods are not really written inline...
class Sys {
private:
int size;
Rad *rad_ptr;
public:
Sys()
{
size=0;
row_ptr=NULL;
};
Sys (int sz)
{
size=sz;
row_ptr=new Rad[size];
};
Sys(Sys &src) //This doesn't work... Why?!?
{
size=src.size;

row_array=new Rad[size];

for(int i; i row_ptr[i]=src.row_ptr[i];
};
~Sys()
{
delete [] row_ptr;
};
}; //End class...

main()
{
Sys s1(10);
Sys s2(10);
Sys s3;

s1.initiate();
s2.initiate();

//This is were I get stuck...
//I want to at least be able
//to copy s1 to s3 (empty),
//but preferably also be able
//to copy s1 to s2 (overwrite).

//These doesn't work... What am I missing?
s2=s1;
s3=s1;
//or...
s2=Sys(s1);
s3=Sys(s1);
//I'm willing to use pointers instead...
//Doesn't work either...
s3ptr=new Sys(s1);

return 0;
}

Answer 1

To your first question, the name acts as an actual instance, just as if you declare a built in type like int or float or char.



The only real problem I see is that you keep using a different name for your array. In the declaration, you call it rad_ptr. Then in some places you call it row_ptr, in others you call it row_array. Make the names consistent and you should be fine.



To overload = properly, write it like this:



Sys & operator=(const Sys & src)

{

if(rad_ptr) delete [ ] rad_ptr;

size = src.size;

rad_ptr = new Rad[size];

for(int i=0; i
return *this;

}



In your destructor, before you delete your array, make sure it's not NULL first, in case it was never initialized.



Not an error, but you can remove the semicolons from the end of the function definitions (but keep the one at the end of the class definition).

Answer 2

First of all, there were several compile errors that needed to be fixed. That should always be your first step before going after the more subtle problems. After fixing those, your code was dumping core for me. I added some cout statements and attributes in the Sys class to see what was going on. See my comments in the code below. As it turns out, your lack of operator= was the main problem.



#include



using namespace std;



class Rad {

private:

    int data[11];

public:

    // Nothing interesting, using default constr, destr and cpyconstr

    // [cja] If you're going to do anything with the data

    // array, you'll need accessors and mutators.

};



// The methods are not really written inline...

class Sys {

private:

    int size;

    Rad *rad_ptr;

    static int instance;

    int id;



public:

    Sys() {

        id = ++instance;

        cout << ". Default constructor (" << id << ")" << endl;

        size = 0;

        rad_ptr = NULL;

    };

    Sys(int sz) {

        id = ++instance;

        cout << ". Full constructor (" << id << ")" << endl;

        size = sz;

        rad_ptr = new Rad[size];

    };



    // This doesn't work... Why?!?

    // [cja] adding const eliminated a compile error;

    Sys(const Sys &src) {

        size = src.size;



        id = ++instance;

        cout << ". Copy constructor, src.id = " << src.id << endl;

        rad_ptr = new Rad[size];



        // [cja] you weren't initializing i, added '= 0'.

        for (int i = 0; i < size; i++) {

            rad_ptr[i] = src.rad_ptr[i];

        }

    };



    // [cja] an assignment operator is what you need, as

    // the previous responder suggested (his code below).

    // This gives you control over the temporary objects

    // being created, and eliminates the core dump I was

    // getting before, caused, it seems, by one of the temp

    // objects being deleted twice.

    Sys &operator=(const Sys &src) {

        cout << "operator= : src.id = " << src.id << endl;

        if(rad_ptr) delete [ ] rad_ptr;

        size = src.size;

        rad_ptr = new Rad[size];

        for(int i=0; i
        return *this;

    }



    ~Sys() {

        cout << ". Destructor, size = " << size

                  << ", id = " << id << endl;

        if (size > 0) {

          delete [] rad_ptr;

        }

    };

}; //End class...



int Sys::instance = 0;



int main(int argc, char *argv[]) {

    Sys s1(10);

    Sys s2(10);

    Sys s3, *s3ptr;



    // s1.initiate();

    // s2.initiate();



    // This is were I get stuck ... I want to at least be able

    // to copy s1 to s3 (empty), but preferably also be able

    // to copy s1 to s2 (overwrite).



    // These don't work... What am I missing?

    cout << "Setting s2 = s1" << endl;

    s2 = s1;

    cout << "Setting s3 = s1" << endl;

    s3 = s1;



    // or...

    cout << "Setting s2 = Sys(s1)" << endl;

    s2 = Sys(s1);

    cout << "Setting s3 = Sys(s1)" << endl;

    s3 = Sys(s1);



    // I'm willing to use pointers instead...

    // Doesn't work either...

    cout << "Setting s3ptr = new Sys(s1)" << endl;

    s3ptr = new Sys(s1);



    cout << "Deleting s3ptr" << endl;

    delete s3ptr;



    cout << "Returning" << endl;

    return 0;

}



#if 0



Output without assignment operator:



. Full constructor (1)

. Full constructor (2)

. Default constructor (3)

Setting s2 = s1

Setting s3 = s1

Setting s2 = Sys(s1)

. Copy constructor, src.id = 1

. Destructor, size = 10, id = 4

Setting s3 = Sys(s1)

. Copy constructor, src.id = 1

. Destructor, size = 10, id = 5

Setting s3ptr = new Sys(s1)

. Copy constructor, src.id = 1

Deleting s3ptr

. Destructor, size = 10, id = 6

Returning

. Destructor, size = 10, id = 5

Aborted (core dumped)





Output with assignment operator:



. Full constructor (1)

. Full constructor (2)

. Default constructor (3)

Setting s2 = s1

operator= : src.id = 1

Setting s3 = s1

operator= : src.id = 1

Setting s2 = Sys(s1)

. Copy constructor, src.id = 1

operator= : src.id = 4

. Destructor, size = 10, id = 4

Setting s3 = Sys(s1)

. Copy constructor, src.id = 1

operator= : src.id = 5

. Destructor, size = 10, id = 5

Setting s3ptr = new Sys(s1)

. Copy constructor, src.id = 1

Deleting s3ptr

. Destructor, size = 10, id = 6

Returning

. Destructor, size = 10, id = 3

. Destructor, size = 10, id = 2

. Destructor, size = 10, id = 1



#endif

Answer 3

Unless you have serious reasons (or it is a school homework), you should not use C arrays in a C++ program. Here's the closest translation of your code to something I could begin working with: (note that user-defined copy constructors and destructors are no longer needed)



#include

using namespace std;



class Rad {

vector data;

public:

Rad() : data(11) {}

};



class Sys {

vector rad_ptr;

public:

Sys() {}

Sys (int sz) : rad_ptr(sz) {}

};



For another important point, unless you have an express purpose for those un-initiated Sys objects, please do not make a constructor that does nothing and a separate Sys::initiate() function that turns your useless default-constructed Sys object into something useful. All that initiation should be made in the constructor, unless there's a serious reason why not.

Answer 4

int main (void) { float d, s1, s2; int *my_array; my_array = new int[50]; //or maybe 'new int[MAX_LENGTH] my_array[33] = 304; printf("33rd position contains %d",my_array[33]); return 0; }