How to use copy constructor with dynamic allocation?

Question

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).

You **really** should not name your variable `new`, that is a C++ keyword. — Cory Kramer, Apr 10 '15 at 13:29
It's not actually called new i just changed their names in English here so they can be more understandable.. — Bonne, Apr 10 '15 at 13:33
You should thoroughly read this: [What is The Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). — πάντα ῥεῖ, Apr 10 '15 at 13:33
After renaming the `new` variable and adding a couple of missing braces, [your code compiles and runs](http://rextester.com/HVXHB84186). What specifically seems to be the problem? — Igor Tandetnik, Apr 10 '15 at 13:35
You should read this too: http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom — andre, Apr 10 '15 at 13:37
When i ran the debugger it showed me that there was something wrong with the copy constructor, but if it's not i really don't know where to look for the mistake. I don't understand why i get bad_alloc error either. (note: this is only a part of the exercise i deleted some parts) — Bonne, Apr 10 '15 at 13:39

score 0 · Accepted Answer · answered Apr 10 '15 at 13:41

0

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);

answered Apr 10 '15 at 13:41

molbdnilo

55,783
3
31
71

When i initialise them in the default constructor do i just give them a value of NULL? – Bonne Apr 10 '15 at 13:50
@Bonne You can do that, as it is safe to delete a null pointer, but then your copy constructor needs to check for null in the original, and you'd probably need to check in more places if the program grows. It's probably simpler to point the name at an empty string or a string that signifies "no name", and `Reg` at some invalid number sequence. Or you could just leave the default constructor out. – molbdnilo Apr 10 '15 at 13:57

How to use copy constructor with dynamic allocation?

1 Answers1