Which version of safe_delete is better?

Question

#define SAFE_DELETE(a) if( (a) != NULL ) delete (a); (a) = NULL;

OR

template<typename T> void safe_delete(T*& a) {
  delete a;
  a = NULL;
}

or any other better way

Answer 1

Generally, prefer inline functions over macros, as macros don't respect scope, and may conflict with some symbols during preprocessing, leading to very strange compile errors.

Of course, sometimes templates and functions won't do, but here this is not the case.

Additionally, the better safe-delete is not necessary, as you could use smart-pointers, therefore not requiring to remember using this method in the client-code, but encapsulating it.

(edit) As others have pointed out, safe-delete is not safe, as even if somebody does not forget to use it, it still may not have the desired effect. So it is actually completely worthless, because using safe_delete correctly needs more thought than just setting to 0 by oneself.

Answer 2

delete a;

ISO C++ specifies, that delete on a NULL pointer just doesn't do anything.

Quote from iso 14882:

    5.3.5 Delete [expr.delete]

    2   [...] In either alternative, if the value of the operand of delete is the 
        null pointer the operation has no effect. [...]

Regards, Bodo

/edit: I didn't notice the a=NULL; in the original post, so new version: delete a; a=NULL; however, the problem with setting a=NULL has already been pointed out (false feeling of security).

Answer 3

You don't need to test for nullity with delete, it is equivalent to a no-op. (a) = NULL makes me lift an eyebrow. The second option is better.

However, if you have a choice, you should use smart pointers, such as std::auto_ptr or tr1::shared_ptr, which already do this for you.

Answer 4

I would say neither, as both will give you a false sense of security. For example, suppose you have a function:

void Func( SomePtr * p ) {
  // stuff
  SafeDelete( p );
}

You set p to NULL, but the copies of p outside the function are unaffected.

However, if you must do this, go with the template - macros will always have the potential for tromping on other names.

Answer 5

Clearly the function, for a simple reason. The macro evaluates its argument multiple times. This can have evil side effects. Also the function can be scoped. Nothing better than that :)

Answer 6

I think

#define SAFE_DELETE(pPtr) { delete pPtr; pPtr = NULL } is better

1. its ok to call delete if pPtr is NULL. So if check is not required.
2. in case if you call SAFE_DELETE(ptr+i), it will result in compilation error.
3. Template definition will create multiple instances of the function for each data type. In my opinion in this case, these multiple definitions donot add any value.
4. Moreover, with template function definition, you have overhead of function call.

Answer 7

As mentioned quite a bit above, the second one is the better one, not a macro with potential unintended side effects, doesn't have the unneeded check against NULL (although I suspect you are doing that as a type check), etc. But neither are promising any safety. If you do use something like tr1::smart_ptr, please make sure you read the docs on them and are sure that it has the right semantics for your task. I just recently had to hunt down and clean up a huge memory leak due to a co-worker putting smart_ptrs into a data structure with circular links :) (he should have used weak_ptrs for back references)

Answer 8

Usage of SAFE_DELETE really appears to be a C programmers approach to commandeering the built in memory management in C++. My question is: Will C++ allow this method of using a SAFE_DELETE on pointers that have been properly encapsulated as Private? Would this macro ONLY work on pointer that are declared Public? OOP BAD!!