1

I have the following method:

class MyClass 
{
public:
    MyClass;

    bool method (MyClass &obj);
};

void MyClass::method (MyClass &obj)
{
    MyClass *c = new MyClass;
    try{
        //code 
        //access another method 
        return true;
    }
    catch (std::string s)
    {
    }
    return false;
}

Where should I delete the pointer c to the object from MyClass: before return true or before return false?

Bo Persson
  • 90,663
  • 31
  • 146
  • 203
sunset
  • 1,359
  • 5
  • 22
  • 35
  • 2
    this question doesn't make any sense. please clarify why you create the object `c`. what's its lifetime? do you need a pointer at all? – Karoly Horvath Aug 29 '11 at 08:12
  • Also, correct the code that is posted, the current code does not really make sense: the `MyClass;` in the class scope does not make sense, there is a missing `;` after the class definition, the semantics of what you intend to do are not clear... – David Rodríguez - dribeas Aug 29 '11 at 08:16
  • possible duplicate of [How to free memory in try-catch blocks?](http://stackoverflow.com/questions/3048377/how-to-free-memory-in-try-catch-blocks) – razlebe Aug 29 '11 at 08:16

5 Answers5

10

What about:

void MyClass::method (MyClass &obj)
{
    MyClass c;
    try{

        //code 
        //access another method 
        return true;
    }
    catch (std::string s)
    {
    }
    return false;
}

No new -> no delete required. c is automatically destroyed when method returns. If your example is oversimplified and you need to create c with new, you should follow the other answers with the smart pointer suggestions.

Stephan
  • 4,187
  • 1
  • 21
  • 33
9

You should be using some form of RAII, So you do not have to bother about deleting the object yourself.

Using RAII, the object itself takes care of deallocating the resources acquired by it, and you do not have to do it yourself.

One of the easiest ways of implementing RAII is using Smart pointers.
You can use unique_ptr.

Community
  • 1
  • 1
Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • 4
    I'm pretty sure this is chinese to the OP – Karoly Horvath Aug 29 '11 at 08:14
  • @yi_H: I linked, *What is RAII?* question in the answer, that explains this in much detail. – Alok Save Aug 29 '11 at 08:16
  • 2
    Of course, in this situation it would probably be best to just stack-allocate the instance of `MyClass`. – Mankarse Aug 29 '11 at 08:19
  • @Mankarse: Ofcourse, If there is no need of dynamic allocation then, One should not be using it at all. But I think the intent of OP in providing that sample code is to provide a minimalistic code sample to ask his doubt & that is not the actual code S/He is using. – Alok Save Aug 29 '11 at 08:21
  • 2
    @yi_H, what if the OP is Chinese ? – iammilind Aug 29 '11 at 08:52
  • It wouldn't be a bad idea to suggest `std::auto_ptr`. I know it's got issues with copying (namely, that it allows copying and it lies when it does so), but if you don't have a `boost::scoped_ptr` or a C++0x `std::unique_ptr`, it makes for a serviceable stack-based pointer. Just don't copy it around. – Nicol Bolas Aug 29 '11 at 08:59
  • Will the Downvoter let know what is wrong in this? Or was S/He just in mood to troll? – Alok Save Aug 30 '11 at 04:07
3

The simple answer is that you should use a smart pointer to manage the memory (the choice of smart pointer depends on the rest of the code), and that will simplify your code. As it is, you would have to delete it in all code paths that exit the context, which includes the two returns and potentially any other exception being thrown.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
3
std::auto_ptr<MyClass> c(new MyClass);
Alex F
  • 42,307
  • 41
  • 144
  • 212
  • 2
    `std::unique_ptr` would be more appropriate in this situation, because `c` is not going anywhere. Using `unique_ptr` states the intent of the code more clearly, and will probably be more efficient. – Mankarse Aug 29 '11 at 08:15
  • 1
    Sometimes I feel that I should downvote each and every answer that without any other explanation or rule, or full knowledge of the problem jump into `shared_ptr`. `shared_ptr` is one of the most restrictive smart pointers in the language, once it claims ownership of a resource, it cannot yield it, which means that if `//code` passes ownership of the pointer to another function the `shared_ptr` will still delete it, or you will be forced (if you can) to change the interface of that other function... @Mankarse is right: `unique_ptr` or even good old `auto_ptr` are better default choices. – David Rodríguez - dribeas Aug 29 '11 at 08:18
  • @David Rodríguez - I used shared_ptr for compatibility with old compilers - not all compilers support C++ 0x. In the context of this question, there is no problem with ownership. For C++ 0x unique_ptr is the answer. – Alex F Aug 29 '11 at 08:25
  • @Alex: old libraries have no `std::shared_ptr`. It is part of C++11, TR1 or boost. – Stephan Aug 29 '11 at 08:57
  • @David Rodríguez - oops, my fault, I wrote shared_ptr, but wanted to use auto_ptr. Yep, my previous comment looks quite stupid :( – Alex F Aug 29 '11 at 10:14
0

You could create a variable outside the try and catch, set it by default to false, instead of return true assign true to it. And instead of the return false return the variable.

You can then delete your instance just before returning that variable.

  • where exactly? it is better to create the instance for myClass inside try? Could you please post your idea? thx – sunset Aug 29 '11 at 08:19