CODEWITHSUNDEEP
c++
user1899020
user1899020

Reputation: 13575

Crash, use it before construction

I have a class, its object will be used by another class as following

class A
{
public:
    void F() {...}
};

class B
{
public:
    void G() { m_a->F(); ...} // use A's function

private:
    A* m_a;
};

Thus it is wrong to call B::G() before the assignment of pointer m_a. Is this acceptable design? Or there is a better way to do it.

The question comes from developing a GUI application. I have a window under the main window to show some basic information of current operation, such as what is happening, how long does it take, and so on. I put this window as a singleton for easy access by the operations which may scatter anywhere, and find the code crashes when closing the GUI application. The crash is due to the window is deleted after the deletion of GUI application (such as qApp in Qt). I then just put a reference pointer of the window in the singleton. And when the window is constructed, I set the reference pointer. The deletion of the window is controlled by the main window. Thus the above mentioned crash problem is solved. But if other developer uses the singleton before the window is constructed, the code will crash too. Any good way to solve it? Or we can just accept it because it is developer's mistake to use it before constructing? Thanks a lot!

Upvotes: 1

Views: 78

Answers (3)

Andy Prowl
Andy Prowl

Reputation: 126452

Invoking function G() may result in Undefined Behavior if m_a is not initialized, so you want to make sure that that's never the case. You have to change your code to make it look a bit like this:

class B
{

public:

    B() : m_a(nullptr) { } // m_a always initialized!

    bool G() 
    { 
        if (m_a == nullptr) return false; // No Undefined Behavior!
        m_a->F(); // This call is safe now
        ...
        return true;
    }

private:

    A* m_a;

};

And by the way, in general you should use smart pointers (choose the type which realizes the appropriate ownership semantics) rather than raw pointers, unless you have a good reason to do that. This would even save you from manual initialization (assuming shared ownership here):

#include <memory>

class B
{

public:

    bool G() 
    { 
        if (m_a == nullptr) return false; // No Undefined Behavior!
        m_a->F(); // This call is safe now
        ...
        return true;
    }

private:

    std::shared_ptr<A> m_a; // Automatically initalized to nullptr

};

Upvotes: 3

twalberg
twalberg

Reputation: 62389

Depending on your expected usage of the class, this can be an acceptable practice, but if you do that, you better change B::G() to check the pointer first:

void G() { if (m_a) m_a->F(); ...}

And make sure to initialize m_a in all constructors, at least to a null pointer if you don't currently have a real A to point at.

Upvotes: 1

Alok Save
Alok Save

Reputation: 206546

You have to use the member initializer list to initialize m_a rather than assign it. It ensures you don't have to bother about getting m_a assigned before calling other functions.

Upvotes: 2

Related Questions