Search code examples
c++classcopy-constructordynamic-allocationbad-alloc

How to use copy constructor with dynamic allocation?


I'm having problems with an exercise for school in which we need to use dynamic allocation for a char array and an int array. The main thing is that I'm not supposed to change the main function and the way the objects are constructed.

class Automobile
{
 char* Name; //this is the name of the car that needs to be saved with dynamic alloc.
 int* Reg; //registration with dynamic alloc.
 int speed; //speed of the car
public:
Automobile(){ speed=0;}
Automobile(char* name,int* Reg,int speed)
{
    Name=new char[strlen(name)+1];
    strcpy(Name,name);
    Reg = new int[5];
    for(int i=0;i<5;i++)
    {
        this->Reg[i]=Reg[i];
    }
    this->speed=speed; //the normal constructor doesn't make any problems since it's called once
}
 Automobile(const Automobile& new)
 {
    Name= new char[strlen(new.Name)+1];
    strcpy(Name,new.Name);
    Reg=new int[5];
    for(int i=0; i<5; i++) Reg[i]=new.Reg[i];
    speed=new.speed;
}

 ~Automobile(){
    delete [] Name;
    delete [] Reg;
}
int main()
{
int n;
cin>>n;

for (int i=0;i<n;i++)
{
    char name[100];
    int reg[5];
    int speed;

    cin>>name;

    for (int i=0;i<5;i++)
        cin>>reg[i];

    cin>>speed;

    Automobile New=Automobile(name,reg,speed);

}

in the main function, the object New is recreated(??) loop so the copy constructor is called(i'm not sure about this). In the copy constructor I don't delete memory(should i?), so the debugger is showing me that there's a problem in the line where i make New Memory for Name. I tried adding delete [] Name and saving the other object's name in a temporary pointer, so i can reappoint the Name to the temporary, but that doesn't work either. The compiler doesn't show any errors when i build it, but the page I'm supposed to be saving the exercise on, shows that i have bad_alloc(i'm not sure if that's connected to the copy pointer).


Solution

  • This, in the three-parameter constructor

    Reg = new int[5];
    

    assigns to the function's parameter, not to the member.
    This leaves the member uninitialised (because you don't initialise it) which causes your copying of the array to write in a random location, which may or may not fail.
    If it doesn't fail, the delete in the destructor is likely to fail.

    A nice fix is to not reuse the names of members for something else in the same scope (rename the parameters, in this case).
    Then leaving out this-> is not only not a disaster, but even recommended.

    You also forgot to initialise the pointer members in the default constructor.

    Side note: the canonical way to create and initialise an object is

    Automobile New(name,reg,speed);