Creating instances to access classes. Invalid parameter passed to C runtime function.

[pepita]

Hi everybody,

How are you? I'm trying to create an instance to a class, to access its members. My application consists in five different classes, acessing one to each other through connections. But I also want to access member variables, so in the past, I declared them as static.
I want to avoid this kind of global variables, and control a little bit the access to them. I am creating a private instance and a reference to it, in the class where the variable is, but then I have the following problems:

QWidget: Must construct a QApplication before a QPaintDevice
Invalid parameter passed to C runtime function.
Invalid parameter passed to C runtime function.

The project is compiling, and in other project that I've done the same, everything is working fine. I post the relevante piece of code:

mainwindow.h

@
class MainWindow : public QMainWindow
{
Q_OBJECT

public:
explicit MainWindow(QWidget *parent = 0);
~MainWindow();

//static int value;
//static QStringList table;

static MainWindow &instance() {return mainwindowInstance;};

signals:
/* [...] */

private slots:
/* [...] */

private:
Ui::MainWindow *ui;
SubWindow *sw;
DialogWindow *dw;
QTimer *timer;
History *history;
Save *save;

static MainWindow mainwindowInstance;
int value;
QStringList table;


friend class DialogWindow;
friend class SubWindow;
friend class History;
friend class Save;

};
@

mainwindow.cpp

@
#include "mainwindow.h"
#include "ui_mainwindow.h"
#include "ui_subwindow.h"
#include "ui_dialogwindow.h"

//int MainWindow::value = 0;
//QStringList MainWindow::table;
MainWindow MainWindow::mainwindowInstance;

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);
setWindowTitle ("MAIN");

sw = new SubWindow (this);
dw = new DialogWindow (this);
history = new History ();
save = new Save ();
timer = new QTimer (this);

/* Connections  [...] */

}

MainWindow::~MainWindow()
{
delete ui;
}

/* Rest of the mainwindow [...] */
@

What I'm doing wrong ?

Thanks for the help.

[Chris Kawa]

@
static MainWindow &instance() {return mainwindowInstance;};
@

This is a very bad bad idea. You're probably calling it somewhere before creating a QApplication instance ( in main() ). Note that this will cause all sorts of undebuggable problems and will most probably crash your app on exit.

It's almost universal rule - never make any kind of QObject static. The order of destruction of these is in many cases undefined and they will crash your app. QObjects (and especially QWidgets) require a living QApplication instance. If you happen ho use one before QApplication is created or after it's destroyed you're asking for trouble.

As for this particular case the usual way to do it is simply this:
@
int main(int argc, char *argv[])
{
QApplication a(argc, argv);
MainWindow w;
w.show();
return a.exec();
}
@
No need for statics or instance() methods here. Lifetimes are defined and predictable. Everything works.

Unrelated observation: why so many dynamically allocated members? Why not simply this:
@
class MainWindow : public QMainWindow
{
...
private:
SubWindow sw;
DialogWindow dw;
QTimer timer;
History history;
Save save;
@
and what is the deal with all those "friends"? Smells like bad design.
Why is a dialog even a memebr at all? It should be created and destroyed where it's needed, not live through the whole lifetime of the main window. Imagine if the OS kept every single possible dialog window in memory all the time. You would need a lot more RAM than you have now.

[pepita]

Wow, so definitely I'm doing the things worse than I thought...

For sure is a bad design, it is a stupid application, which I do use to train and familiarize myself with signals and slots, and access between classes (and almost everything...).

So, and because I'm pursuing good design (even in a stupid app), could you please tell me what is the correct and clean (and safe) way to access class member variables and functions from outside the class, but avoiding static variables?

And what about accessing private members without declaring a friend? Would I need an instance to the class, and an access function to the private member?

Here is the whole code. What the application does is basically, in three different type windows (that's why I'm using a dialog), do exactly the same, and what is done in one of them, should be shown in the rest.

mainwindow.h
@
namespace Ui
{
class MainWindow;
}

class MainWindow : public QMainWindow
{
Q_OBJECT

public:
explicit MainWindow(QWidget *parent = 0);
~MainWindow();

static int value;

signals:
void sendText (QString);
void sendValue (QString);

private slots:
void on_pushButtonIncrease_clicked();
void on_pushButtonTest_clicked();
void on_pushButtonDialog_clicked();
void on_pushButtonSubwindow_clicked();
void timerDelay();
void startTemp();
void populate(QString text);

private:
Ui::MainWindow *ui;
SubWindow *sw;
DialogWindow *dw;
QTimer *timer;
History *history;
Save *save;
};
@

subwindow.h
@
namespace Ui
{
class SubWindow;
}

class SubWindow : public QMainWindow
{
Q_OBJECT

public:
explicit SubWindow(QWidget *parent = 0);
~SubWindow();

signals:
void openDialog();
void startTemp();
void sendText (QString);
void sendValue (QString);

private slots:
void on_pushButtonIncrease_clicked();
void on_pushButtonTest_clicked();
void on_pushButtonDialog_clicked();
void populate(QString text);

private:
Ui::SubWindow *ui;
friend class MainWindow;
};
@

dialogwindow.h
@
namespace Ui
{
class DialogWindow;
}

class DialogWindow : public QDialog
{
Q_OBJECT

public:
explicit DialogWindow(QWidget *parent = 0);
~DialogWindow();

signals:
void openSubwindow();
void startTemp();
void sendText (QString);
void sendValue (QString);

private slots:
void on_pushButtonIncrease_clicked();
void on_pushButtonTest_clicked();
void on_pushButtonSubwindow_clicked();
void populate(QString text);

private:
Ui::DialogWindow *ui;
friend class MainWindow;
};

#endif // DIALOGWINDOW_H
@

saveclass.h
@
class Save : public QObject
{
Q_OBJECT

public:
Save();

static QStringList table;

signals:
void sendAck (QString);

public slots:
void saveTxt ();
};

#endif // SAVECLASS_H
@

mainwindow.cpp
@
#include "mainwindow.h"
#include "ui_mainwindow.h"
#include "ui_subwindow.h"
#include "ui_dialogwindow.h"

int MainWindow::value;

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);
setWindowTitle ("MAIN (QMainWindow)");

sw = new SubWindow (this);
dw = new DialogWindow (this);
history = new History ();
save = new Save ();
timer = new QTimer (this);

/*  CONNECTIONS */

/* Let both SubWindow and DialogWindow open each other */
/* Send text and values from the MainWindow to the rest */
/* Send text and values from the SubWindow to the rest */
/* Send text and values from the Dialog to the rest */
/* Send typed text to create history list */
/* Receive back the created history list and populate the list views */
/* Save the history table into .txt file when pressing the save button */
/* Save the history table into .txt file when writing the keyword 'Save' */
/* Send ACK when the .txt file is saved to the history */
/* Set timer to reset the labels on all windows after 5 seconds */

}

MainWindow::~MainWindow()
{
delete ui;
}

void MainWindow::on_pushButtonIncrease_clicked()
{
value++;
ui->labelMain->setText ("Value: " + QString("%1").arg(value));
emit sendValue ("Value: " + QString("%1").arg(value));

startTemp ();

}

void MainWindow::on_pushButtonTest_clicked()
{
ui->labelMain->setText ("Text: " + ui->lineEdit->text ());
emit sendText ("Text: " + ui->lineEdit->text ());

startTemp ();

}

void MainWindow::on_pushButtonDialog_clicked()
{
dw->show();
}

void MainWindow::on_pushButtonSubwindow_clicked()
{
sw->show();
}

void MainWindow::startTemp()
{
timer->start(5000);
}

void MainWindow::timerDelay ()
{
timer->stop ();
ui->labelMain->setText ("Increased value or Test text will apear here...");
emit sendText ("Increased value or Test text will apear here...");
}

void MainWindow::populate(QString text)
{
ui->listWidgetMain->addItem (text);
Save::table << text;
}
@

Rest of the .cpp [...]

saveclass.cpp
@
#include "saveclass.h"
#include "mainwindow.h"

QStringList Save::table;

Save::Save()
{
}

void Save::saveTxt ()
{
QString outputFilename = QFileDialog::getSaveFileName (0, "Save file...", QDir::currentPath(), "Text Files(*.txt)");;
QFile outputFile(outputFilename);
outputFile.open(QIODevice::WriteOnly);

if(!outputFile.isOpen())
    emit sendAck ("File:     Error ocurred");
else
    emit sendAck ("File:     Saved successfuly");

QTextStream outStream(&outputFile);

for(int row = 0; row < table.count (); row++)
{
    QString textLine = table [row];
    outStream << textLine << endl;
}

outputFile.close();

}
@

[ the rest of .cpp files]

I've removed all the connections and other things to be able to post within the characters limit, but anyways, sorry for the annoying code and thanks once again.

Best regards.

[Chris Kawa]

Ok, so let's start by clearing up what static means. It means that all instances of the class will have a single, common member that will live for all the eternity(well, until the app dies anyway). If you change it from one instance it will be changed for all of them. If you change it via global accessor (MainWindow::value) it will be changed for all instances. So there's no difference here between a static member and a global variable somewhere outside of the class, except it can be made private or protected.
In your case you've got only one instance of MainWindow so there's absolutely no point in having that member static.

Now to the point of passing values and accessing class members. Either make the member public or (some people prefer that because of OO princibles) via getter/setter methods. I'll give a small example that you can hopefully translate to your own code. It will also show you how you can use dialogs locally, without keeping track of all those ugly pointers everywhere.

@
//dialog.h
class Dialog : public QDialog {
//...
public:
void setInputStuff(int value);
SomeType getResultStuff() const;
};

//dialog.cpp
void Dialog::setInputStuff(int value) {
//do something with value, eg.
ui->someSpinbox->setValue(value);
}

SomeType Dialog::getResultStuff() const {
auto result = constructResultSomehowWhateverThatIs();
return result;
}

//mainwindow.h
class MainWindow : public QMainWindow {
//...
public:
//these two are not even used in this example but here's how you do it:
inline int value() const { return value; }
inline void setValue(int newValue) { value = newValue; }

void processSomeStuffFromDialog();

private:
int value;
};

//mainwindow.cpp
//#include "dialog.h"

void Mainwindow::processSomeStuffFromDialog() {
//That's one way or you could even make the input a constructor param:
//Dialog dialog(value);
Dialog dialog;
dialog.setInputStuff(value);

dialog.exec();
auto stuff = dialog.getResultStuff();
//do something with stuff...
}
@

This way Dialog class doesn't even know something like MainWindow exists (why should it?!), so you can easily change or reuse it without impact on the rest of the app.
MainWindow doesn't include dialog.h header in the mainwindow.h header and pollute the whole app.
MainWindow doesn't keep track of any Dialog pointers. Just creates a local instance and gets what it needs from it. Very much more maintainable. If you want to change something processSomeStuffFromDialog() is the only place you touch. Don't have to go hunting for Dialog pointers.

[pepita]

Thanks for the tips.

I will put them into practice, and I will try to stick to this cleaner and safer programming.

Best regards.