1

The other day I did a C ++ interview. I had to do code review...

This is not the example from interview but it is a good example of my question

class A{
    public:
    A(int n) : m_n{n}, m_v{new char[n]}{
    //do something
    }
    ~A(){
        if(m_v != nullptr)
        {
            //here I said it should be checked and initialized with nullptr
            delete []m_v;
            m_v = nullptr;
        }
    }
    private:
    int m_n;    
    char *m_v;

};

The interviewer said it was useless to do that. Nothing happens if you delete nullptr. I expected it to be undefined behavior. So my question Is a good practice to do that or not ? (or is it mandatory)

Ionut Alexandru
  • 680
  • 5
  • 17
  • Well, I'd say that's a case for coding style. – Kamiccolo Apr 25 '20 at 21:44
  • `delete[] m_v;` properly handles the nullptr case. Setting `m_v = nullptr;` is of no actual value since the object is just about to be a non-object... however, I do that in my dev debug code because it helps to expose dangling pointers. (Not that I'd ever have a dangling pointer.) – Eljay Apr 25 '20 at 21:44
  • 5
    Your interviewer was right. This is completely useless. In other situations this may not be, but in this case it is 100% useless. It is logically impossible, in a well-defined C++ program, to get the result of `delete`ing the pointer that that above code has set to a `nullptr` value. Therefore, this is logically useless. Again, this statement applies explicitly, and specifically, to this exact use case and only to this exact use case, and to nothing else. – Sam Varshavchik Apr 25 '20 at 21:45
  • Some references: [Deleting nullptr - performance overhead?](https://stackoverflow.com/questions/9741520/deleting-nullptr-performance-overhead) * [Is it still safe to delete nullptr in c++0x?](https://stackoverflow.com/questions/6731331/is-it-still-safe-to-delete-nullptr-in-c0x) * [What C++17 standard say about calling delete on nullptr?](https://stackoverflow.com/questions/47354881/what-c17-standard-say-about-calling-delete-on-nullptr) * [Is it worth setting pointers to NULL in a destructor?](https://stackoverflow.com/questions/3060006/is-it-worth-setting-pointers-to-null-in-a-destructor) – JaMiT Apr 25 '20 at 21:54
  • Your dtor is private in your example. So, you cannot define `A a;`. You only can define `A *a;`. and in this case you cannot call delete on `a` because the dtor is private. – TonySalimi Apr 25 '20 at 21:59

1 Answers1

4

Both, checking for nullptr and assigning nullptr after the delete are not needed and hence should not be there in this example.

From cppreference on delete (emphasize mine):

For the first (non-array) form, expression must be a pointer to an object type or a class type contextually implicitly convertible to such pointer, and its value must be either null or pointer to a non-array object created by a new-expression, or a pointer to a base subobject of a non-array object created by a new-expression.

[...]

If expression is not a null pointer and the deallocation function is not a destroying delete (since C++20), the delete expression invokes the destructor (if any) for the object that's being destroyed, or for every element of the array being destroyed (proceeding from the last element to the first element of the array).

Calling delete on a nullptr does nothing.

Assigning nullptr after delete in your example is pointless unless there is other code in the destructor that checks m_v for being a nullptr or not. After the destructor is run m_v does not exist anymore.

PS: This really isn't about style. Code that has no chance to do anything useful should not be written. Really no offense, but doing the check for nullptr and setting the pointer to nullptr can be considered as cargo cult programming.

Community
  • 1
  • 1
463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185