Problems with Copy Constructor - Homework Problem -

[D0CHollywood]

I am having big time issues understanding the copy constructor when a pointer is involved. My code works, but then crashes at the end. I have attempted to debug with VS2015, but cannot figure out the issue. Any help would be appreciated!

  // Specification file for Car Class
#ifndef CAR_H
#define CAR_H

class Car
{

private:
   char* description; //description of car as a char pointer
   int arraySize;

public:
   Car(char*); //Constructor one. Receives int size and char *description of car and dynamically allocates a char array from it
   Car(const Car &); //Copy Constructor 
   char* getDescription() const; //Accessor getDescription method
   void setDescription(char*); //Mutator setDescription method
   void operator= (const Car &);
   ~Car(); // Car Destructor

};

#endif






// Implementation file for Car class
#include "Car.h"  
#include <string>
#include<iostream>
using namespace std;

// CONSTRUCTOR  
Car::Car(char* desc)
{
   arraySize = strlen(desc); // get size of passed in string and save to arraysize int variable
   description = new char[arraySize+1]; //add 1 for null terminator 
   strcpy(description, desc);
}

// COPY CONSTRUCTOR
Car::Car(const Car &obj)
{
   arraySize = obj.arraySize;
   description = new char[arraySize+1];
   strcpy(description, obj.description);
}

// DESTRUCTOR for description
Car::~Car()
{
   cout << "Freeing memory...";
   delete [] description; // delete dynamically allocated memory for description
}


char* Car::getDescription() const
{
   return description;
}

void Car::setDescription(char *mainInput)
{
   description = mainInput;
}


// MAIN FUNCTION
#include<iostream>
#include<string>
#include "Car.h"
using namespace std;

int main()
{

   Car userCar1( "Honda Accord"); //use first car constructor that takes char array 
   cout << "The description of Car 1 is: " << userCar1.getDescription() << endl;

   Car userCar2 = userCar1; //Call copy constructor to copy userCar1 into Car2
   cout << "The description of Car 2 is: " << userCar2.getDescription() << endl;

   userCar1.setDescription("Ford Focus"); //Now set car 1 to Ford Focus

   cout << "The new description of Car 1 is: " << userCar1.getDescription() << endl;
   cout << "The description of Car 2 is: " << userCar2.getDescription() << endl;
   userCar2.~Car(); // Run destructor to free dynamically allocated memory for car2

   return 0;
}

[keskiverto]

Do not call the destructor explicitly; it is now called twice.

[AbstractionAnon]

In addition to what keskiverto mentioned, you have an issue at line 46.

You're trying to delete an uninitialized pointer.

I don't know if you've learned std::string yet, but using std::string is a lot easier than trying to manage dynamically allocated C-strings yourself.

[D0CHollywood]

Thanks guys. I changed the destructor to only delete userCar2 at the end. I would love to use the string class, but my assignment requires a dynamically allocated C string.

Although I changed the destructor to only be called once, I am still getting an error at the end.

Visual studio shows this as the error:
"Expression:_CrtIsValidHeapPointer(block)"

Up until the crash, the output is shown properly.

[AbstractionAnon]

Did you update your code in your original post?

Line 46 still shows an attempt to delete an uninitialized pointer.
Lines 90-91 still show explicit invocation of your destructors.

You also have a problem as line 67. When setDescription() is called from line 86, "Ford Focus" is not a dynamically allocated string. It is a pointer to a const string. You copy that const pointer in description. Then when userCar1 is destructed, you attempt to release description, which is a copy of the pointer to the const string. You can't release a string that wasn't dynamically allocated.

[D0CHollywood]

Thanks for the help. I edited the original code.

You said that "Ford Focus" is not a dynamically allocated string, so I tried to change my setDescription function to create it as such and received the same error. I have honestly read the same 5 pages of our text about copy constructors 20 times, but am clearly just confused!

Am I not even dynamically allocating the string "Honda Accord" correctly?

[keskiverto]

There is nothing wrong in your copy constructor. There never was.

You do still call explicitly the destructor for 'userCar2' on line 89. That is an error. Remove that. You will still see the output from the destructors of both userCar1 and userCar2, because the destructor of local variables is called automatically at the end of the scope.


The other error is in Car::setDescription, line 66.
It should deallocate the old array (Car::description) and allocate new space, but only if the current array is too small for the new string (that is pointed to by the 'mainInput' pointer).
Then copy the string, just like your constructors do.


Your member functions that do take a pointer argument should check whether the pointer's value is null. Someone could do:
Car fubar( nullptr );
That will call strlen( 0 ); which produces undefined behaviour.


Your compiler should warn about lines 79 and 85, because you call functions that take char* with const string literals. Those functions should take const char *. They will not modify their arguments, just copy from them.

[D0CHollywood]

@Keskiverto Thanks so much. If you didnt clarify that the destructor is AUTOMATICALLY called, I don't believe I would have understood that. That will certainly help for my exam in two weeks!

Got the code working with all of your help. Thanks! Much Appreciated!