can a pointer be deleted several times c++?

Question

If I have the following example:

test.h

class MyClass
{
public:
MyClass();
std::string name1;
std::string name2;
std::string type1;
std::string type2;

void method1(MyClass &obj1);
void method2(MyClass &obj2);
}

test.cpp

MyClass *mainObject = new MyClass();

MyClass::MyClass()
{
}
void MyClass::method1((MyClass &obj1)
{
//do stuff
mainObject=&obj1; //we populate some of the MyClass variables


}
void MyClass::method2((MyClass &obj2)
{
//do stuff
mainObject=&obj2; //we populate the rest of MyClass variables
}

When should I delete mainObject inside test.cpp? Should I create a destructor in order for the client to delete it?

Your code doesn't make any sense. You are creating a global heap-allocated `MyClass` object, and then modifying it via non-static member functions. This is a massive memory leak. What are you trying to do here? — Oliver Charlesworth, Aug 23 '11 at 14:30
I want an instance of the class MyClass that will be modified during different methods' compilation and will be updated every time. So each time I'm appealing a new method mainObect will always have the last update — sunset, Aug 23 '11 at 14:34
The code is quite dubious. In `MyClass::method1`, you have **two** objects. There's `obj1`, but also `*this`. The comment "some of the MyClass variables" doesn't make sense in that light. Both objects have their own variables. — MSalters, Aug 23 '11 at 14:35
@sunset: But you have three objects involved! You have the object that `mainObject` points to, you have the object that you're calling `method1()` on, and you have `obj1`. — Oliver Charlesworth, Aug 23 '11 at 14:44
should I change the mainObject pointer with MyClass mainObject?In this case I guess there will not be a memory leak.Am I right? — sunset, Aug 23 '11 at 14:47
@sunset: That may very well be the case; check my second answer. I've added code that reflects my guess as to what you exactly want. Also, I think my crystal ball needs new batteries now. — MSalters, Aug 23 '11 at 14:54

score 4 · Answer 1 · answered Aug 23 '11 at 14:32

4

This is a good example that's best solved by not thinking about it yourself.

Use a shared_ptr<MyClass> mainObject; (either the new C++11 or the Boost version). It will do the delete for you.

Mind you, method1() and method2() should take their argument by shared_ptr too. Currently, they're doing a very bad thing: deleting an object that's passed by reference.

answered Aug 23 '11 at 14:32

MSalters

173,980
10
155
350

No, they are currently not deleting anything. What they do is making wild assumptions about the lifetime of an object that is passed in via reference. (Apart from the syntax errors, that is.) – Christopher Creutzig Aug 23 '11 at 14:35
currently i didn't delete the mainobject. Now I want to optimize my code. That's why I am asking for soluions. THX. APPRECIATE. Is there another alternative besides using smart pointers? – sunset Aug 23 '11 at 14:37
@sunset: As I said in my answer, the important thing is the program logic, not the code: Who is responsible for that object? If that piece of code is done with it, the object should be deleted. No sooner, no later. Ownership can also be passed around, of course, but if there is no clear ownership, it's probably a good idea to redesign, instead of throwing smart pointer magic at the problem and hoping it will go away. – Christopher Creutzig Aug 23 '11 at 14:40
@sunset: smart pointers aren't magical. You could write equivalent code yourself. Even if you got it right, though, they'd be far harder to understand. The existing `shared_ptr<>` behavior is very well documented. – MSalters Aug 23 '11 at 14:42
could you please give me some ideas in my case? Is there a way of not using smart pointers? The thing is that I need this global object. Should I have the pointer with: MyClass mainObject;? I need to finish this as soon as possible. Appreciate! – sunset Aug 23 '11 at 14:43
@sunset: Christopher Creutzig does have a point. You're going too fast with not enough understanding, neither of C++ nor of your own problem domain. For instance, you say "this global object". Which one? `MyClass *mainObject` defines a global pointer, not an object. – MSalters Aug 23 '11 at 14:48
I thought about replacing *mainObject with MyClass mainObject; Will this solve the problem? – sunset Aug 23 '11 at 14:54

score 2 · Answer 2 · edited May 23 '17 at 12:26

2

Deleting a pointer variable (pointing to non-0) several times is worse than not deleting it. Because the former can cause hard to find bugs and undefined behavior.

Your code is not correctly written. You should delete mainObject; as soon as you try to assign it with &obj1 or &obj2. But make sure that you do it only first time. Don't delete the pointer if it's pointing to obj1 or obj2.

I feel from this question and previous question of yours, that you are coming from Java/C# background. Better to read a good book on C++ first, you will learn that most of the time you don't need new/delete.

edited May 23 '17 at 12:26

Community

1
1

answered Aug 23 '11 at 14:32

iammilind

68,093
33
169
336

1

Unless you set it to NULL each time after you delete it, in that case you can keep on deleting it forever :) – Ioan Paul Pirau Aug 23 '11 at 14:33
what do you mean by . You should delete mainObject; as soon as you try to assign it with &obj1 or &obj2? can you please write me an example? – sunset Aug 23 '11 at 14:39

score 1 · Answer 3 · answered Aug 23 '11 at 14:32

You should delete the pointer when you are done using the object it points to. You should not delete a pointer twice while it is pointing to a single object. You should not delete a pointer if it is pointing to an object that you didn't dynamically allocate with new.

score 0 · Answer 4 · answered Aug 23 '11 at 14:33

There should always be a logical owner of any resource, and that owner should delete the resource.

There are cases where it makes sense to have shared ownership, and that is what boost::shared_ptr and similar solutions are for. The last one to give up ownership is then the one to delete the resource.

Simon · Answer 5 · 2011-08-23T20:05:35.537

When should I delete mainObject inside test.cpp?

When it is no longer used.

Should I create a destructor in order for the client to delete it?

You only have to create a destructor if some resources of class MyClass have to be released - this is not the case with the shown code. The one you should release (=delete) is mainObject. But anyway, method1(..) and method2(..) are overwriting the mainObject pointer which leads to a dangling pointer (you can't reach the object anymore).

[EDIT]
To answer your question:

can a pointer be deleted several times c++?

Pointers are typically not allocated with new - only the objects they pointing to.
If you mean "can delete be called several times on the same pointer?" the answer is no and would lead to UB. delete on a pointer which is zero is defined and legal.

@sunset: Why wannt you have the state of a MyClass object to be shared? According your first comment i think it would be better to pass around a reference / pointer instead of using the shared state (which introduces great hidden dependecies). -> No static / global objects - no matter whether newed or not. — Simon, Aug 23 '11 at 15:22

score 0 · Answer 6 · answered Aug 23 '11 at 14:53

From all comments it looks like you might actually want the following:

static MyClass mainObject; // Not a pointer. Local to test.cpp

void MyClass::method1()
{
  //do stuff
  mainObject=*this; // Make a copy of the last object modified.
}
void MyClass::method2()
{
  //do stuff
  mainObject=*this; // Make a copy of the last object modified.
}

In this way, whether you call foo.method1() or bar.method2, the object on the left side of the . is copied to mainObject. No pointer funkyness needed at all, no new and no delete.

score 0 · Accepted Answer · answered Aug 23 '11 at 16:46

I think that I'd go a slightly different way.

Like this:

test.h

class MyClass
{
public:
  MyClass();
  std::string name1;
  std::string name2;
  std::string type1;
  std::string type2;

  void method1(MyClass &obj1);
  void method2(MyClass &obj2);
}

test.cpp

MyClass mainObject; // default c-tor called automatically.

MyClass::MyClass()
{
}
void MyClass::method1(MyClass & obj1)
{
  //do stuff

  //we populate some of the MyClass variables
  mainObject.name1=obj1.name1;
  mainObject.type1=obj2.type1;
}
void MyClass::method2(MyClass & obj2)
{
  //do stuff

  //we populate more of the MyClass variables
  mainObject.name2=obj1.name2;
  mainObject.type2=obj2.type2;
}

There is no simple way to only populate part of your object without specifying which parts.

But, otherwise, if you don't make mainObject a pointer then you don't need to allocate space for it, that's done automatically. (But, I should object to use of globals unless they are REALLY needed!)

This implementation of what I THINK you're trying to do will completely avoid the need for use of the heap, no need for new/delete.

can a pointer be deleted several times c++?

7 Answers7