0

I am trying to create two classes whose instances get created and deleted together. One class is a base for the other:

class Interface;

class MyClass
{
friend class Interface;
private:
  MyClass() {}
public:
  static MyClass *NewInstance();
  Interface *ControlPanel;
};

class Interface : public MyClass
{
friend class MyClass;
private:
  Interface() {}
public:
  void Control1() {cout << "control1" << endl;}
  void Control2() {cout << "control2" << endl;}
  void Control3() {cout << "control3" << endl;}
};

The two member functions that are supposed to create and delete instances are:

MyClass *MyClass::NewInstance()
{
  MyClass *inst = new MyClass;
  inst->ControlPanel = new Interface;
  return inst;
}

void DeleteMyClassInstance(MyClass *inst)
{
  delete inst->ControlPanel;
  inst->ControlPanel = 0;
  delete inst;
  inst = 0;
}

I had success in linking the instance creation process with the use of a function in the base class (NewInstance()) which creates the instances. But the deletion function (DeleteMyClassInstance()) doesn't work (that is, I can still use both inst1 and inst1->ControlPanel after calling the function):

int main()
{
  MyClass *inst1 = MyClass::NewInstance();

  inst1->ControlPanel->Control1();

  DeleteMyClassInstance(inst1);

  inst1->ControlPanel->Control1();

  return 0;
}

But if I put the deletion code inside the main function, it works perfectly (the inst1->ControlPanel->Control1() statement that comes after the delete statements doesn't work, and that's what I want):

int main()
{
  MyClass *inst1 = MyClass::NewInstance();

  inst1->ControlPanel->Control1();

  delete inst->ControlPanel;
  inst->ControlPanel = 0;
  delete inst;
  inst = 0;

  inst1->ControlPanel->Control1();

  return 0;
}

My question is: Why does putting the delete statements directly inside the main function work while putting them inside a separate function and using it in main doesn't? Why does the code in my DeleteMyClassInstance() function get ignored by the compiler?

Mo Sanei
  • 445
  • 6
  • 22
  • This sounds like a horribly bastardised implementation of Singletons - don't do it, just use shared pointers if you're worried about memory management. – The Forest And The Trees Mar 25 '13 at 09:51
  • 2
    @TheForestAndtheTrees: it doesn't seem to me to have anything to do with Singletons. As many instances of the class can be created as you like by calling `NewInstance` multiple times. – Steve Jessop Mar 25 '13 at 09:54

2 Answers2

2

The main difference is that with the code in the main function inst=0 sets the variable in the main function to null. With the code in DeleteMyInstance, the line inst=0 only sets the local variable in DeleteMyInstance to null (uselessly, since it is unused after that point -- enable more warnings and your compiler might mention it). It doesn't affect the completely separate variable of the same name in main.

So, your code

DeleteMyClassInstance(inst1);
inst1->ControlPanel->Control1();

has undefined behavior because you attempt to use an object that has already been deleted. UB means that anything can happen. If it appears to work that is probably because in your implementation the function Control1 still "works" even when called on a null (or otherwise-invalid) pointer, since the function doesn't use this or any data members. But that implementation detail should not be relied on.

Be aware (if you aren't already) that your code shows some bad C++ style. You should not write special functions to delete objects owned by your class, that's what destructors are for. And you shouldn't have to explicitly delete objects in destructors, that's what smart pointers are for. And you shouldn't use dynamic allocation when not needed, that's what automatic variables and data members are for. By all means get this code right once, as a learning exercise what goes on behind the scenes, but that should be with a view to doing it properly as soon as possible.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
  • Thanks. You're perfectly right about the coding style but I just wanted to to know if that was possible. – Mo Sanei Mar 25 '13 at 12:39
1

Change your DeleteMyClassInstance function to.

void DeleteMyClassInstance(MyClass **inst)
{
  delete (*inst)->ControlPanel;
  (*inst)->ControlPanel = 0;
  delete (*inst);
  *inst = 0;
}
Premsuraj
  • 2,506
  • 1
  • 17
  • 16