Deleting an uninitialised object in C++ [duplicate]

This question already has answers here: Is it safe to delete a NULL pointer? 开发者_C百科 (8 answers) Closed 2 years ago.

In my header file I declare a variable within the scope of a class:

    FaultModel<double>   *fm_req_set_odom_px;

...which is conditionally initialised in the class constructor, depending on the value of a configuration file:

    const char *configModel = ConfigReader->ReadString("FaultModel");
    if (strcmp(configModel, "cyclic") == 0)
        fm_req_set_odom_px = new CyclicFaultModel<double>();

My question is: do I need to wrap the delete with a conditional to check if the model was initialised or not, or is it safe to just delete it in either case?

   if (fm_req_set_odom_px != NULL) // Is this necessary?
       delete fm_req_set_odom_px;

---

Apart from other answers which guide you appropriately,

If you MUST use dynamically allocated objects then do not use raw pointers but use Smart pointers.

Always make use of RAII(SBRM) it makes your life easier. This way you don't have to bother explicitly deleting any resources, the resources themselves will take care of it

---

delete NULL; is guaranteed to be a no-op, so a manual check is not necessary. However, an unitialized pointer variable is not NULL, so you must explicitly set it to NULL if the condition fails:

if (strcmp(configModel, "cyclic") == 0)
    fm_req_set_odom_px = new CyclicFaultModel<double>();
else
    fm_req_set_odom_px = NULL;

Alternatively, you can set the pointer variable to NULL unconditionally before the if statement:

fm_req_set_odom_px = NULL;
if (strcmp(configModel, "cyclic") == 0)
    fm_req_set_odom_px = new CyclicFaultModel<double>();

---

There is no need for the check. But better approach is to use a smart pointer...

---

Typically, you would instead use something like a boost::variant to hold the possibilities. Heap-based dynamic allocation is wasteful in this case. boost::variant will always appropriately destroy it's contents for you and you won't have to worry about writing your own copy constructor/assignment operator.

---

delete NULL;

has no effect, so the test is not necessary.

---

I think a more fundamental question to ask first is: Are you initializing the pointer to NULL in your constructor? (i.e. using an initializer list for instance).

If you are not initializing it to NULL, then you will most likely get access violations as you attempt to delete random bits of memory. If you are initializing it to NULL, then you can skip the if guard statement.

---

delete checks for NULL, there is no need for if (myVar != NULL) delete myVar;. Also, consider using std::auto_ptr (or some other smart pointer container). This gets you out of a lot of troubles, e.g. uninitialized pointers.