CODEWITHSUNDEEP
c++exceptiondestructor
DNamto
DNamto

Reputation: 1362

Exception Error in Destructor

I know this is very basic but somehow working on different technologies I have mashed up my C++ concepts

I have created a simple program but it is giving exception when the destructor is called.

Below is the code:

#include "stdafx.h"
#include<iostream>

using namespace std;

class Employee
{
  public:

    Employee(char *name){
      cout<<"Employee::ctor\n";
      myName_p = new char(sizeof(strlen(name)));
      myName_p = name;
    }

    void disp(){cout<<myName_p;}

    ~Employee()
    {
      cout<<"Employee:dtor\n\n";
      delete myName_p;
    }

  private:
    char *myName_p;
};


int main()
{
    Employee emp("check");
    emp.disp();
    return(0);
}

Requesting all to clear this basic concept. As per my understanding we can't use delete[] because we are not using new[] in this case. Though I have tried using delete[] , but still it was giving error

Upvotes: 0

Views: 210

Answers (2)

Yang
Yang

Reputation: 8180

Change 

myName_p = new char(sizeof(strlen(name)));
myName_p = name;

to

myName_p = strdup(name);

and #include <cstring>. This creates new space and copies the parameter string. In this way, you'll have to call free instead of delete in your destructor.

Otherwise, after the second assignment, you have assigned the string literal "check" to myName_p, and the newly created space is discarded. Then your destructor tries to delete "check" rather than the allocated space, which results in crash.

Also, it is better practice to use std::string rather than old char* strings:

class Employee
{
public:
  Employee(char *name): myName_p(name) {
    cout<<"Employee::ctor\n";
  }

  void disp(){ cout << myName_p; }

private:
  std::string myName_p;
};

The string class will manage memory for you.

Upvotes: 2

Mats Petersson
Mats Petersson

Reputation: 129524

You REALLY should use std::string here.

It is so much easier, especially for a beginner. The list of errors is:

  1. you are calculating the wrong size of the name, it should be strlen(name)+1, not using sizeof anything.
  2. You also should use new char[strlen(name)+1].
  3. You are copying the data from the string supplied as the argument to the constructor, use strcpy rather than name_p = name - the latter leaks the memory you just allocated, and then you have a pointer to a const char * which you should not delete.
  4. If you fix the allocation so that it is correct, you should then use delete [] name_p;.

However, it you instead use std::string, all of the above problems go away completely, you can just do:

Employee(char *name) name_p(name) { ... }

and get rid of all the problematic new, delete and copying. Of course, name_p is probably no longer a suitable name for the variable, but you get the idea.

Upvotes: 7

Related Questions