Destructor of object cause crashing

Question

I'm a noob in c++. I have a problem that I cannot solve anyway. I wrote a code to understand better classes and overload operator:

#include <iostream>
#include <stdlib.h>
#include <stdarg.h>
using namespace std;

class vectmy {
public:
    int size;   
int *a;
vectmy(int n,int val);
~vectmy (){delete[] a;   //IF I DELETE THIS PROGRAM WORKS
}
vectmy & operator = (const vectmy &);
};

 vectmy::vectmy(int n,int val){
    size=n;
    a = new int[ n+1 ];
    for (int i=0;i<n;++i){
    *(a+i)=val;
    }
 }



   vectmy& vectmy::operator= (const vectmy& param)
  {
   for (int i=0;i<3;++i)    a[i]=param.a[i];
    return *this;
   }



 vectmy operator+( vectmy left, vectmy right)
  {
  vectmy result = left;
  for (int i=0;i<3;++i) result.a[i]=result.a[i]+right.a[i];

  return result;
   }



int main() {
int b1[3]={1,2,4};
vectmy d(3,2),b(3,4),c(3,0);

c=(b+d);

for (int j=0; j<3; ++j)cout<<c.a[j]<<' '<<endl; 
return 0;
}

When I run it crashes. If I remove the destructor it works. Why it happens?

Follow the [rule of three](http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29). You need a copy constructor. — juanchopanza, Apr 20 '13 at 11:30
possible duplicate of [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — juanchopanza, Apr 20 '13 at 11:33

score 4 · Accepted Answer · edited May 23 '17 at 11:43

When I run it crashes. If I remove the destructor it works. Why it happens?

Your operator + creates a copy of left here:

vectmy result = left;

Since you haven't defined a copy constructor explicitly, the compiler will generate one implicitly that performs member-wise copying.

A dull copy of the a data member means that the a pointer will eventually point to the same location for two different instances of vectmy (result and left), both of which will delete[] it upon destruction.

Such double deletion gives your program undefined behavior, which in your case manifests itself as a crash.

This is the point of the Rule of Three: every time you have a user-defined copy-constructor, assignment operator, or destructor, you should probably define all of them.

The reason is that you normally define one of those functions because you are managing some resource (memory, in your case), and you usually want to perform proper actions when copying, destructing, or assigning objects that manage a resource.

In this particular case, a proper copy constructor is missing. This is how you could define it:

vectmy::vectmy(vectmy const& v)
{
    size=v.size;
    a = new int[size];
    *this = v;
}

Also, I would suggest you to avoid manual memory management through raw pointers, new, and delete (or their array counterparts) whenever you can, and consider using std::vector instead.

UPDATE:

Also notice, that your operator + is accepting its parameters by value, which means that each argument will be copied (i.e., the copy constructor will be invoked).

Since a copy is not really necessary here, you may want to take your parameters by reference (to const):

vectmy operator + ( vectmy const& left, vectmy const& right)
//                         ^^^^^^              ^^^^^^

Really good answer, thank you very much! Surely I'll use std::vector, it was only a test to see how classes with pointer data member behave. — The Newbie Toad, Apr 20 '13 at 11:43
However i tried with this operator insted:`vectmy vectmy::operator + (const vectmy param) { vectmy temp(3,0); for (int i=0;i<3;++i) temp.a[i]=a[i]+param.a[i]; return temp; }` And I see that it works with your copy costructor, not without, although temp is create and not copied by another! Could you clarify this more? Thanks! — The Newbie Toad, Apr 20 '13 at 12:03
@TommasoFerrari: You're welcome, good luck with your project ;) — Andy Prowl, Apr 20 '13 at 12:26

Some programmer dude · Answer 2 · 2013-04-20T11:37:03.463

In your operator+ you do

vectmy result = left;

This will call the default constructor and copy constructor, of which you have none. So the compilers variants will be used, which doesn't allocate memory for the a member. For the automatically generated copy constructor the pointer will simply be copied, making two object use the same pointer. When one deletes it, the other pointer becomes invalid.

You should read about the rule of three.

Destructor of object cause crashing

2 Answers2