Destructor problem

[Zivojin]

What is wrong with this destructor?

  Workflow::~Workflow()
{
	for(int i=0;i<this->duz;i++)
		delete p[i];//Here the program fails
	delete[] p;
}

Unhandled exception at 0x0081D301 in Projekat4.exe: 0xC0000005: Access violation reading location 0xDDDDDDE5.

This is constructor

Workflow::Workflow(int a)
{
	duz = a;
	p = new Step*[duz];
}

p is array of pointers type Step(abstract class).

[Repeater]

You should call delete only on something that was created using new.

delete p[i]; //Here the program fails
This suggests that whatever pointer is in p[i], it was NOT created using new.

The constructor definitely is not populating the array with Step-pointers from a new. Show us the code that puts Step-pointer objects in the array.



Also, you should not be using new at all and should not be using delete at all, but that's a separate problem.

[coder777]

You should have somewhere a line like

p[...] = new Step{...};

Currently

p = new Step*[duz];

creates an array of uninitialized pointer. Change it to

p = new Step*[duz]{}; // Note: {}

in order to initialize them. Then the delete will not crash.

[salem c]

http://www.cplusplus.com/forum/beginner/247737/
It looks like you failed to comprehend properly the meaning of deep copy (what you need) vs shallow copy (what you have).

Workflow::Workflow(const Workflow & w)
{
	duz = w.duz;
}

All you did was make copies of a bunch of pointers.

Which means when you do this in one copy.

Workflow::~Workflow()
{
	for(int i=0;i<this->duz;i++)
		delete p[i];//Here the program fails
	delete[] p;
}

You're really deleting the pointers which exist in all other copies as well.

So when you go to destruct those other copies, the memory has already been freed.

So the system barfs all over you.

[Zivojin]

Define fuction defines Steps as Activity or State.provera=0 is a private member of Workflow .in my main

Workflow *W1 = new Workflow(2018);
	W1->Define("Activity", 1009);
	W1->Define("State", 1009);

void Workflow::Define(const char * tip,int broj)
{
	int s=0;
	int i;
	int f=0;
	if (0==strcmp(tip,"Activity"))
		s = 1;
	if (0 == strcmp(tip, "State"))
		s = 2;
	while ((s == 1) && (provera < duz) && (f<broj))
	{ 
		p[provera] = new Activity;
		provera++;
		f++;
		p[provera] = new State;
	}
	while ((s == 2)&&( provera < duz) && (f<broj))
	{
		provera++;
		f++;
	}


}

Yes,i read about deep copy.I have corrected that im my code

Workflow::Workflow(const Workflow & w)
{
	duz = w.duz;
	if (w.p)
	{
		p = new Step*[duz];
		for (int i = 0; i < duz; i++)
			p[i] = w.p[i];
	}
	else
		p = 0;
}

I am using Steps only to achieve polimorphism because that is our task:Ussualy we have array of Pointers to some object and we are meant to achieve polymorphism.