Search code examples
c++singletonwxwidgets

C++: Single instance of a dialog?


I have a dialog box I want to only exist once, as two different classes (MainFrame, which can open it outright, and Wizard, which can open it with settings brought in from the wizard itself) have to access it at any given time. Ideally, I would like the user to be able to open the dialog with the Wizard, with everything filled out, then close it. If they need to change settings, they should be able to open it with the MainFrame and edit it there.

The way I have it implemented currently has two different instances of the dialog box. If it's opened from the MainFrame after the Wizard, the Mainframe one will have default values, while the Wizard one can also be open with values from the wizard.

I tried implementing singleton design pattern following this but attempting to open the Wizard or the dialog itself causes it to give an xC0000005 error.

MainFrame has access to both the Wizard and Dialog headers, and the Dialog has access to the Wizard header for the function that passes data from the Wizard to the Dialog (with Dialog forward-declared in Wizard's header).

I'm really not sure of the way to go forward with this, but any help is appreciated. I can give code on my implementation if necessary.

Edit: Didn't do much outside of the linked URL, but it is developed on wxWidgets if that makes any difference. Here's relevant code.

Dialog.h

#include "Wizard.h"
class Dialog
{
private:
    Dialog() {}
    Dialog(Dialog const& copy) {}
    Dialog& operator=(Dialog const& copy) {}

    static Dialog* instance;

//many functions and variables

public:
    Dialog(wxWindow* parent) //more fields, generated by wxFormBuilder
    ~Dialog();

    Wizard* wizard;
    void SetRelative(Wizard* inWizard);

    static Dialog* GetInstance()
    {
        instance = new Dialog(NULL);
        return instance;
    }
};
Dialog.cpp

#include "Dialog.h"

Dialog* Dialog::instance = 0;

Dialog::Dialog(wxWindow parent, etc.)
{
    //lots of stuff
}

//event handlers, helper functions

void Dialog::SetRelative(Wizard* inWizard)
{
    this->Wizard = inWizard
}
Wizard.h

class Dialog;

class Wizard
{
public:
    Dialog* dialog;
}
Wizard.cpp

#include "WizardSetup.h"
#include "WizardDialog.h"

Wizard::Wizard(wxWindow* parent, etc.)
{
//bunch of stuff
    dialog->GetInstance()
    dialog->SetRelative(this) <--------- hangs at this statement
}
MainFrame.cpp

#include "Dialog.h"
#include "Wizard.h"

class MainFrame: public wxFrame
{
protected:
    Dialog* m_dialog;
    Wizard* m_wizard;

etc.
}
MainFrame.cpp
#include "MainFrame.h"

MainFrame::MainFrame(wxWindow* parent, etc.)
{
//stuff here

m_dialog->GetInstance();
}

void MainFrame::menuDialogOnMenuSelection( wxCommandEvent& event)
{
    m_dialog->Show(); <-------- hangs here
}

Solution

  • Three (immediate) issues with this code:

    1. Your Dialog::Dialog(WxWindow*) constructor is public, which sends mixed signals about the nature of your singleton object. While not directly responsible for crashing or hanging, you're exposing your class to improper use that may violate the singleton contract (and thus indirectly cause crashes, hangs, or worse: silent errors) when The Next Guy (which might be you in a month) tries to maintain this code.

    2. You're not checking for the existence of an already instantiated Dialog in your Dialog::GetInstance() function. You should have

    static Dialog* GetInstance()
    {
        if (instance == nullptr) instance = new Dialog(nullptr);
        return instance;
    }
    
    1. You're not assigning the various Dialog* member variables (such as Wizard::dialog and MainFrame::m_dialog) the address of your singleton Dialog. Be sure to do that:
    // (in the Wizard constructor for example)
    dialog = Dialog::GetInstance();
    

    Addendum note: I think it's worth mentioning that the singleton pattern is often called an antipattern. I'm not smart enough to understand why. It assumedly tends to cause some undesirable coding practices, and it's definitely cumbersome to deal with sometimes, but hey, whatever gets the job done. Just keep in mind that it's not necessarily the best tool for the job according to Some People.