Trying to remove circular dependency.

[Scriptdaemon]

I have two classes that interact in a way that would cause a circular dependency.

Say I have one class, called TaskBarIcon, where a pointer is created in a class called MainDialog, thus MainDialog needs TaskBarIcon. This is necessary so that when I destroy MainDialog I can delete the pointer to TaskBarIcon. However, TaskBarIcon uses functions defined in MainDialog (that can't easily or practically be separated into their own class), so TaskBarIcon also needs MainDialog.

I'm wondering if there is a better way to refactor my code so that this is no longer a problem.


// Name: MainDialog.h
// TODO: Clean up code

#ifndef __CMAINDIALOG_H__
#define __CMAINDIALOG_H__

#ifdef __BORLANDC__
#pragma hdrstop
#endif

#ifndef WX_PRECOMP
#include <wx/wx.h>
#include <wx/dialog.h>
#else
#include <wx/wxprec.h>
#endif

////Header Include Start
#include <wx/dirdlg.h>
#include <wx/spinctrl.h>
#include <wx/combobox.h>
#include <wx/stattext.h>
#include <wx/checkbox.h>
#include <wx/textctrl.h>
#include <wx/statbox.h>
#include <wx/button.h>
////Header Include End

#include <stdio.h>
#include <time.h>
#include <wx/dir.h>
#include <wx/bitmap.h>
#include <wx/fileconf.h>
#include <wx/gdicmn.h>
#include <wx/msgdlg.h>
#include <wx/stdpaths.h>
#include "FileConfig.h"
#include "TaskBarIcon.h"
#include "WindowsApi.h"

////Dialog Style Start
#undef CMainDialog_STYLE
#define CMainDialog_STYLE wxCAPTION | wxSYSTEM_MENU | wxDIALOG_NO_PARENT | wxCLOSE_BOX
////Dialog Style End

class CMainDialog : public wxDialog
{
private:
DECLARE_EVENT_TABLE();

public:
CMainDialog(wxWindow *parent, wxWindowID id = 1, const wxString &title = wxT("BGRandomize"), const wxPoint &pos = wxDefaultPosition, const wxSize &size = wxDefaultSize, long style = CMainDialog_STYLE);
~CMainDialog();
void OnButtonClickBrowse(wxCommandEvent&);
void OnButtonClickChangeBackground(wxCommandEvent&);
void OnButtonClickClose(wxCommandEvent&);
void OnCheckBoxClickAutoStart(wxCommandEvent&);
void OnCheckBoxClickSubFolders(wxCommandEvent&);
void OnCheckBoxClickHiddenFiles(wxCommandEvent&);
void OnComboBoxSelectLargeImageStyle(wxCommandEvent&);
void OnComboBoxSelectMediumImageStyle(wxCommandEvent&);
void OnComboBoxSelectSmallImageStyle(wxCommandEvent&);
void OnSpinCtrlUpdateSmallImageSize(wxSpinEvent&);
void ChangeBackground();
void RefreshBackground();

private:
////GUI Control Declaration Start
wxDirDialog *BrowseBGDlg;
wxCheckBox *AutoStartCheckBox;
wxCheckBox *HiddenFilesCheckBox;
wxSpinCtrl *SmallImageSizeSpinCtrl;
wxButton *BrowseButton;
wxComboBox *SmallImageStyleComboBox;
wxStaticText *WxStaticText3;
wxComboBox *MediumImageStyleComboBox;
wxStaticText *WxStaticText2;
wxComboBox *LargeImageStyleComboBox;
wxStaticText *WxStaticText1;
wxStaticBox *WxStaticBox2;
wxCheckBox *SubFoldersCheckBox;
wxTextCtrl *BGDirTxtCtrl;
wxStaticBox *wxStaticBox1;
wxButton *CloseButton;
wxButton *ChangeBackgroundButton;
////GUI Control Declaration End

private:
enum
{
////GUI Enum Control ID Start
ID_AUTOSTARTCHECKBOX = 1016,
ID_HIDDENFILESCHECKBOX = 1015,
ID_SMALLIMAGESIZESPINCTRL = 1014,
ID_BROWSEBUTTON = 1013,
ID_SMALLIMAGESTYLECOMBOBOX = 1012,
ID_WXSTATICTEXT3 = 1011,
ID_MEDIUMIMAGESTYLECOMBOBOX = 1010,
ID_WXSTATICTEXT2 = 1009,
ID_LARGEIMAGESTYLECOMBOBOX = 1008,
ID_WXSTATICTEXT1 = 1007,
ID_WXSTATICBOX2 = 1006,
ID_SUBFOLDERSCHECKBOX = 1005,
ID_BGDIRTXTCTRL = 1004,
ID_WXSTATICBOX1 = 1003,
ID_CLOSEBUTTON = 1002,
ID_CHANGEBACKGROUNDBUTTON = 1001,
////GUI Enum Control ID End
};

private:
void CreateGUIControls();
void OnClose(wxCloseEvent&);
CTaskBarIcon *m_pTaskBarIcon;
};

#endif



// Name: MainDialog.cpp
// TODO: Clean up code

#include "MainDialog.h"

////Event Table Start
BEGIN_EVENT_TABLE(CMainDialog,wxDialog)
////Manual Code Start
////Manual Code End

EVT_CLOSE(CMainDialog::OnClose)
EVT_CHECKBOX(ID_AUTOSTARTCHECKBOX,CMainDialog::OnCheckBoxClickAutoStart)
EVT_CHECKBOX(ID_HIDDENFILESCHECKBOX,CMainDialog::OnCheckBoxClickHiddenFiles)
EVT_SPINCTRL(ID_SMALLIMAGESIZESPINCTRL,CMainDialog::OnSpinCtrlUpdateSmallImageSize)
EVT_BUTTON(ID_BROWSEBUTTON,CMainDialog::OnButtonClickBrowse)
EVT_COMBOBOX(ID_SMALLIMAGESTYLECOMBOBOX,CMainDialog::OnComboBoxSelectSmallImageStyle)
EVT_COMBOBOX(ID_MEDIUMIMAGESTYLECOMBOBOX,CMainDialog::OnComboBoxSelectMediumImageStyle)
EVT_COMBOBOX(ID_LARGEIMAGESTYLECOMBOBOX,CMainDialog::OnComboBoxSelectLargeImageStyle)
EVT_CHECKBOX(ID_SUBFOLDERSCHECKBOX,CMainDialog::OnCheckBoxClickSubFolders)
EVT_BUTTON(ID_CLOSEBUTTON,CMainDialog::OnButtonClickClose)
EVT_BUTTON(ID_CHANGEBACKGROUNDBUTTON,CMainDialog::OnButtonClickChangeBackground)
END_EVENT_TABLE()
////Event Table End

CMainDialog::CMainDialog(wxWindow *parent, wxWindowID id, const wxString &title, const wxPoint &position, const wxSize& size, long style)
: wxDialog(parent, id, title, position, size, style)
{
CreateGUIControls();
}

CMainDialog::~CMainDialog()
{
delete m_pTaskBarIcon;
}

void CMainDialog::CreateGUIControls()
{
////GUI Items Creation Start

BrowseBGDlg = new wxDirDialog(this, wxT("Choose a directory"), wxT(""));

AutoStartCheckBox = new wxCheckBox(this, ID_AUTOSTARTCHECKBOX, wxT("Automatically start with Windows"), wxPoint(21, 92), wxSize(252, 17), 0, wxDefaultValidator, wxT("AutoStartCheckBox"));

HiddenFilesCheckBox = new wxCheckBox(this, ID_HIDDENFILESCHECKBOX, wxT("Include hidden files"), wxPoint(153, 45), wxSize(109, 17), 0, wxDefaultValidator, wxT("HiddenFilesCheckBox"));

SmallImageSizeSpinCtrl = new wxSpinCtrl(this, ID_SMALLIMAGESIZESPINCTRL, wxT("128"), wxPoint(113, 163), wxSize(59, 22), wxSP_ARROW_KEYS, 128, 768, 128);

BrowseButton = new wxButton(this, ID_BROWSEBUTTON, wxT("..."), wxPoint(242, 21), wxSize(31, 21), 0, wxDefaultValidator, wxT("BrowseButton"));

wxArrayString arrayStringFor_SmallImageStyleComboBox;
arrayStringFor_SmallImageStyleComboBox.Add(wxT("Stretched"));
arrayStringFor_SmallImageStyleComboBox.Add(wxT("Centered"));
arrayStringFor_SmallImageStyleComboBox.Add(wxT("Tiled"));
SmallImageStyleComboBox = new wxComboBox(this, ID_SMALLIMAGESTYLECOMBOBOX, wxT("Tiled"), wxPoint(177, 164), wxSize(96, 21), arrayStringFor_SmallImageStyleComboBox, wxCB_READONLY, wxDefaultValidator, wxT("SmallImageStyleComboBox"));

WxStaticText3 = new wxStaticText(this, ID_WXSTATICTEXT3, wxT("For images <= to:"), wxPoint(21, 167), wxDefaultSize, 0, wxT("WxStaticText3"));

wxArrayString arrayStringFor_MediumImageStyleComboBox;
arrayStringFor_MediumImageStyleComboBox.Add(wxT("Stretched"));
arrayStringFor_MediumImageStyleComboBox.Add(wxT("Centered"));
arrayStringFor_MediumImageStyleComboBox.Add(wxT("Tiled"));
MediumImageStyleComboBox = new wxComboBox(this, ID_MEDIUMIMAGESTYLECOMBOBOX, wxT("Centered"), wxPoint(177, 138), wxSize(96, 21), arrayStringFor_MediumImageStyleComboBox, wxCB_READONLY, wxDefaultValidator, wxT("MediumImageStyleComboBox"));

WxStaticText2 = new wxStaticText(this, ID_WXSTATICTEXT2, wxT("For images smaller than screen:"), wxPoint(21, 141), wxDefaultSize, 0, wxT("WxStaticText2"));

wxArrayString arrayStringFor_LargeImageStyleComboBox;
arrayStringFor_LargeImageStyleComboBox.Add(wxT("Stretched"));
arrayStringFor_LargeImageStyleComboBox.Add(wxT("Centered"));
arrayStringFor_LargeImageStyleComboBox.Add(wxT("Tiled"));
LargeImageStyleComboBox = new wxComboBox(this, ID_LARGEIMAGESTYLECOMBOBOX, wxT("Stretched"), wxPoint(177, 112), wxSize(96, 21), arrayStringFor_LargeImageStyleComboBox, wxCB_READONLY, wxDefaultValidator, wxT("LargeImageStyleComboBox"));

WxStaticText1 = new wxStaticText(this, ID_WXSTATICTEXT1, wxT("For images larger than screen:"), wxPoint(21, 115), wxDefaultSize, 0, wxT("WxStaticText1"));

WxStaticBox2 = new wxStaticBox(this, ID_WXSTATICBOX2, wxT("Behavior"), wxPoint(9, 76), wxSize(276, 119));

SubFoldersCheckBox = new wxCheckBox(this, ID_SUBFOLDERSCHECKBOX, wxT("Include subfolders"), wxPoint(31, 45), wxSize(109, 17), 0, wxDefaultValidator, wxT("SubFoldersCheckBox"));

BGDirTxtCtrl = new wxTextCtrl(this, ID_BGDIRTXTCTRL, wxT(""), wxPoint(21, 21), wxSize(216, 21), 0, wxDefaultValidator, wxT("BGDirTxtCtrl"));

wxStaticBox1 = new wxStaticBox(this, ID_WXSTATICBOX1, wxT("Background Folder"), wxPoint(9, 3), wxSize(276, 69));

CloseButton = new wxButton(this, ID_CLOSEBUTTON, wxT("Close"), wxPoint(157, 207), wxSize(118, 25), 0, wxDefaultValidator, wxT("CloseButton"));

ChangeBackgroundButton = new wxButton(this, ID_CHANGEBACKGROUNDBUTTON, wxT("Change Background"), wxPoint(19, 207), wxSize(118, 25), 0, wxDefaultValidator, wxT("ChangeBackgroundButton"));

SetTitle(wxT("BGRandomize"));
SetIcon(wxNullIcon);
SetSize(8,8,300,266);
Center();

////GUI Items Creation End

AutoStartCheckBox->SetValue(CFileConfig::Instance()->Read(wxT("Settings/AutoStart"), (long)0));
SubFoldersCheckBox->SetValue(CFileConfig::Instance()->Read(wxT("Settings/SubFolders"), (long)0));
HiddenFilesCheckBox->SetValue(CFileConfig::Instance()->Read(wxT("Settings/HiddenFiles"), (long)0));

LargeImageStyleComboBox->SetValue(CFileConfig::Instance()->Read(wxT("Settings/LargeImageStyle"), wxT("Stretched")));
MediumImageStyleComboBox->SetValue(CFileConfig::Instance()->Read(wxT("Settings/MediumImageStyle"), wxT("Centered")));
SmallImageStyleComboBox->SetValue(CFileConfig::Instance()->Read(wxT("Settings/SmallImageStyle"), wxT("Tiled")));

SmallImageSizeSpinCtrl->SetValue(CFileConfig::Instance()->Read(wxT("Settings/SmallImageSize"), 128));

BGDirTxtCtrl->SetValue(CFileConfig::Instance()->Read(wxT("Settings/BackgroundFolder"), CWindowsApi::GetPicturesPath()));

//CFileConfig::Instance()->Read(wxT("Settings/HideOnLaunch"), wxT("False"));

CAboutDialog *pAboutDialog = new CAboutDialog(NULL);

// Initialize the task bar icon
m_pTaskBarIcon = new CTaskBarIcon(this, pAboutDialog);
}

void CMainDialog::OnClose(wxCloseEvent&)
{
CFileConfig::Instance()->Flush();
Show(false);
}

void CMainDialog::OnButtonClickBrowse(wxCommandEvent&)
{
BrowseBGDlg->SetPath(BGDirTxtCtrl->GetValue());
if (BrowseBGDlg->ShowModal() == wxID_OK) {
BGDirTxtCtrl->SetValue(BrowseBGDlg->GetPath());
CFileConfig::Instance()->Write(
wxT("Settings/BackgroundFolder"),
BrowseBGDlg->GetPath()
);
}
}

void CMainDialog::OnButtonClickChangeBackground(wxCommandEvent&)
{
ChangeBackground();
}

// Change to OnButtonClickHide();
void CMainDialog::OnButtonClickClose(wxCommandEvent&)
{
CFileConfig::Instance()->Flush();
Show(false);
}

void CMainDialog::OnCheckBoxClickAutoStart(wxCommandEvent&)
{
if (AutoStartCheckBox->GetValue()) {
CWindowsApi::AddRunItem();
} else {
CWindowsApi::RemoveRunItem();
}
CFileConfig::Instance()->Write(
wxT("Settings/AutoStart"),
AutoStartCheckBox->GetValue()
);
}

void CMainDialog::OnCheckBoxClickSubFolders(wxCommandEvent&)
{
CFileConfig::Instance()->Write(
wxT("Settings/SubFolders"),
SubFoldersCheckBox->GetValue()
);
}

void CMainDialog::OnCheckBoxClickHiddenFiles(wxCommandEvent&)
{
CFileConfig::Instance()->Write(
wxT("Settings/HiddenFiles"),
HiddenFilesCheckBox->GetValue()
);
}

void CMainDialog::OnComboBoxSelectLargeImageStyle(wxCommandEvent&)
{
CFileConfig::Instance()->Write(
wxT("Settings/LargeImageStyle"),
LargeImageStyleComboBox->GetValue()
);
}

void CMainDialog::OnComboBoxSelectMediumImageStyle(wxCommandEvent&)
{
CFileConfig::Instance()->Write(
wxT("Settings/MediumImageStyle"),
MediumImageStyleComboBox->GetValue()
);
}

void CMainDialog::OnComboBoxSelectSmallImageStyle(wxCommandEvent&)
{
CFileConfig::Instance()->Write(
wxT("Settings/SmallImageStyle"),
SmallImageStyleComboBox->GetValue()
);
}

void CMainDialog::OnSpinCtrlUpdateSmallImageSize(wxSpinEvent&)
{
CFileConfig::Instance()->Write(
wxT("Settings/SmallImageSize"),
SmallImageSizeSpinCtrl->GetValue()
);
}

void CMainDialog::ChangeBackground()
{
if (wxDir::Exists(BGDirTxtCtrl->GetValue()) == false) {
wxMessageBox(wxT("This directory does not exist."), wxT("BGRandomize Warning"), wxOK | wxICON_EXCLAMATION);
return;
}

wxString szBmpPath = CWindowsApi::GetPicturesPath() + wxT("\\BGRandomize.bmp");

int iFlags = wxDIR_FILES;
if (SubFoldersCheckBox->GetValue()) {
iFlags |= wxDIR_DIRS;
}
if (HiddenFilesCheckBox->GetValue()) {
iFlags |= wxDIR_HIDDEN;
}
wxArrayString szFiles;
wxDir::GetAllFiles(BGDirTxtCtrl->GetValue(), &szFiles, wxT("*.bmp"), iFlags);
wxDir::GetAllFiles(BGDirTxtCtrl->GetValue(), &szFiles, wxT("*.gif"), iFlags);
wxDir::GetAllFiles(BGDirTxtCtrl->GetValue(), &szFiles, wxT("*.jpg"), iFlags);
wxDir::GetAllFiles(BGDirTxtCtrl->GetValue(), &szFiles, wxT("*.png"), iFlags);

// TODO: Check if files exist and are in array
szFiles.Remove(CFileConfig::Instance()->Read(wxT("Settings/LastImageShown"), wxT("")));
szFiles.Remove(szBmpPath);
szFiles.Shrink();

if (szFiles.GetCount() == 0) {
wxMessageBox(wxT("There are no image files in this directory."), wxT("BGRandomize Warning"), wxOK | wxICON_EXCLAMATION);
return;
}

int iScreenW, iScreenH;
const wxString szFile = szFiles[rand() % szFiles.GetCount()];
wxString szStyle = MediumImageStyleComboBox->GetValue();
wxBitmap bmpImage(szFile, wxBITMAP_TYPE_ANY);
wxDisplaySize(&iScreenW, &iScreenH);
if (bmpImage.GetWidth() > iScreenW || bmpImage.GetHeight() > iScreenH) {
szStyle = LargeImageStyleComboBox->GetValue();
} else if (bmpImage.GetWidth() <= SmallImageSizeSpinCtrl->GetValue() && bmpImage.GetHeight() <= SmallImageSizeSpinCtrl->GetValue()) {
szStyle = SmallImageStyleComboBox->GetValue();
}
CWindowsApi::SetBackground(bmpImage, szStyle);
}

void CMainDialog::RefreshBackground()
{
// TODO
}



// Name: TaskBarIcon.h
// TODO: Clean up code

#ifndef __CTASKBARICON_H__
#define __CTASKBARICON_H__

#ifdef __BORLANDC__
#pragma hdrstop
#endif

#ifndef WX_PRECOMP
#include <wx/wx.h>
#include <wx/dialog.h>
#else
#include <wx/wxprec.h>
#endif

#include <wx/msgdlg.h>
#include <wx/taskbar.h>
#include "AboutDialog.h"
#include "FileConfig.h"
#include "MainDialog.h"

class CTaskBarIcon : public wxTaskBarIcon
{
public:
CTaskBarIcon(CMainDialog *pMainDialog, CAboutDialog *pAboutDialog);
virtual wxMenu *CreatePopupMenu();
void OnMenuClickChange(wxCommandEvent&);
void OnMenuClickRefresh(wxCommandEvent&);
void OnMenuClickShow(wxCommandEvent&);
void OnMenuClickAbout(wxCommandEvent&);
void OnMenuClickQuit(wxCommandEvent&);
void OnTaskBarDClick(wxTaskBarIconEvent&);

private:
enum {
E_CHANGE = 1005,
E_REFRESH = 1004,
E_SHOW = 1003,
E_ABOUT = 1002,
E_QUIT = 1001
};

// Dialog members
CAboutDialog *m_pAboutDialog;
CMainDialog *m_pMainDialog;

DECLARE_EVENT_TABLE();
void OnInit(CMainDialog *pMainDialog, CAboutDialog *pAboutDialog);
};

#endif



// Name: TaskBarIcon.cpp
// TODO: Clean up code

#include "TaskBarIcon.h"

// --------------------------------------------------------------------------
// Public functions
// --------------------------------------------------------------------------

CTaskBarIcon::CTaskBarIcon(
CMainDialog *pMainDialog,
CAboutDialog *pAboutDialog
)
{
OnInit(pMainDialog, pAboutDialog);
}

/* virtual */ wxMenu *CTaskBarIcon::CreatePopupMenu()
{
wxMenu *pMenu = new wxMenu;
pMenu->Append(E_CHANGE, wxT("&Change Background"));
pMenu->Append(E_REFRESH, wxT("&Refresh Background"));
pMenu->AppendSeparator();
pMenu->Append(E_SHOW, wxT("&Show Settings"));
pMenu->Append(E_ABOUT, wxT("&About"));
pMenu->AppendSeparator();
pMenu->Append(E_QUIT, wxT("&Quit"));
return pMenu;
}

void CTaskBarIcon::OnMenuClickChange(wxCommandEvent&)
{
m_pMainDialog->ChangeBackground();
}

void CTaskBarIcon::OnMenuClickRefresh(wxCommandEvent&)
{
m_pMainDialog->RefreshBackground();
}

void CTaskBarIcon::OnMenuClickShow(wxCommandEvent&)
{
m_pMainDialog->Show();
}

void CTaskBarIcon::OnMenuClickAbout(wxCommandEvent&)
{
m_pAboutDialog->Show();
}

void CTaskBarIcon::OnMenuClickQuit(wxCommandEvent&)
{
CFileConfig::Instance()->Flush();
m_pAboutDialog->Destroy();
m_pMainDialog->Destroy();
}

void CTaskBarIcon::OnTaskBarDClick(wxTaskBarIconEvent&)
{
m_pMainDialog->Show();
}

// --------------------------------------------------------------------------
// Private functions
// --------------------------------------------------------------------------

BEGIN_EVENT_TABLE(CTaskBarIcon, wxTaskBarIcon)
EVT_MENU(E_CHANGE, CTaskBarIcon::OnMenuClickChange)
EVT_MENU(E_REFRESH, CTaskBarIcon::OnMenuClickRefresh)
EVT_MENU(E_SHOW, CTaskBarIcon::OnMenuClickShow)
EVT_MENU(E_ABOUT, CTaskBarIcon::OnMenuClickAbout)
EVT_MENU(E_QUIT, CTaskBarIcon::OnMenuClickQuit)
EVT_TASKBAR_LEFT_DCLICK(CTaskBarIcon::OnTaskBarDClick)
END_EVENT_TABLE()

/**
* Initializes the task bar icon
*
* @param pMainDialog Pointer for the main dialog
* @param pAboutDialog Pointer for the about dialog
*/
void CTaskBarIcon::OnInit(
CMainDialog *pMainDialog,
CAboutDialog *pAboutDialog
)
{
m_pAboutDialog = pAboutDialog;
m_pMainDialog = pMainDialog;
if (SetIcon(wxNullIcon, wxT("BGRandomize")) == false) {
wxMessageBox(
wxT("Unable to set task bar icon."),
wxT("BGRandomize Warning"),
wxOK | wxICON_EXCLAMATION
);
}
}

[oracleguy]

Just forward declare the main dialog in the taskbar icon header file. Then you won't need to include the main dialog header file.


// Name: TaskBarIcon.h
// TODO: Clean up code

#ifndef __CTASKBARICON_H__
#define __CTASKBARICON_H__

class CMainDialog;

#ifdef __BORLANDC__
#pragma hdrstop
#endif

#ifndef WX_PRECOMP
#include <wx/wx.h>
#include <wx/dialog.h>
#else
#include <wx/wxprec.h>
#endif


You could rework it so the taskbar icon uses callback functions instead. Either way would work.

[Scriptdaemon]

I've tried that before, but when I do I keep getting "invalid use of undefined type `struct CMainDialog'" errors.

Edit: I just looked at your blog. The program I'm working on is a lot like your desktop background switcher.

[oracleguy]

Are you including the main dialog header file in TaskBarIcon.cpp? Sorry, I didn't think to mention that earlier to make sure you would. You can forward delcare it for the header file but in the .cpp you still need to bring in the definition of the main dialog.

[Scriptdaemon]

Ah, alright. Now it works (though it seems like an ugly solution, so I'll look into callback functions like you also mentioned later).

Thanks.