jryannel / qt5-cadaques

Repo for Qt5 Cadaques Project
26 stars 4 forks source link

Outdated/poor examples for QtWidgets module. #268

Open Seally opened 10 years ago

Seally commented 10 years ago

Since the section instructs the reader to use Qt 5, the example given under 'Widget Application' is incorrect (but is correct for Qt 4). QWidget has been moved to QtWidgets in Qt 5 and merely including QtGui will not allow this code to compile.

Also, you can just simply put QWidget on the stack instead of bothering with QScopedPointer (or std::unique_ptr and similar smart pointers) as it will be automatically destroyed anyway.

The correct code should be somewhere along the lines of:

// Don't use #include <QtWidgets> if you can avoid it.
#include <QApplication>
#include <QWidget> // Replace with #include "customwidget.h" if appropriate.

int main(int argc, char* argv[]) {
    QApplication app(argc, argv);

    // Don't use a pointer. Include it on the stack.
    QWidget widget; // Replace with CustomWidget widget; if appropriate.
    widget.resize(240, 120);
    widget.show();

    return app.exec();
}

In the next example, #include <QtWidgets> is highly NOT recommended as a header file include as it will make the resulting binary gigantic for no good reason. Breaking this into:

#include <QWidget>
#include <QPoint>

is a much better solution.

Later examples should forward declare classes (if possible) or add additional #includes as needed so readers won't supersize their binaries.

On a completely unrelated note: I hope QMLBook is not abandoned yet. It's a great primer to QML, despite its flaws.

back-link: ch02/index.html#widget-application

ghost commented 10 years ago

Really? Have you added QT += widgets in the .pro file?

Seally commented 10 years ago

The problem isn't whether or not I can get the example to work (I can), but I find the wording to be problematic because:

  1. As I've stated, the example establishes poor programming practices (including 'QtWidgets' includes almost every, if not every, header available in the QtWidgets module, inflating the compiled binary for no good reason). I have pointed out that including just 'QApplication' and 'QWidget' is sufficient for the example in question and advised that this be changed.
  2. Here I'll just review the example code on site. For convenience, here's the copy-paste:
#include <QtGui>

int main(int argc, char** argv)
{
    QApplication app(argc, argv);
    QScopedPointer<QWidget> widget(new CustomWidget());
    widget->resize(240, 120);
    widget->show();
    return app.exec();
}
    1. The line #include <QtGui> is useless here in the case of Qt 5 as QApplication is now part of QtWidgets and not of QtGui anymore. All of QApplication's dependencies are already included in the header file, and unless one uses them explicitly as well, it is simply not recommended to add them. #include <QApplication> is a far better option and will actually compile after customwidget.h is also included.
    2. As we're assuming that readers are new to this, putting a #include "customwidget.h" line at the top could clarify a couple of things and prevent some headscratches. It's fine to leave this line out after a couple more examples but I find this to be too early.
    3. I see no need for QScopedPointer as CustomWidget widget; could just as easily replace this entire line with some -> to . replacements in the later lines.

Now that you mention it, it would also be nice to mention the QtWidgets module separation from QtGui during the Qt 4->5 transition* and that the line greaterThan(QT_MAJOR_VERSION, 4): QT += widgets (or the slightly less portable QT += widgets) can used to remedy "QWidget not found" and similar errors. I won't rely on the IDE to put this in for me. I still remember myself a couple of years back spending ages trying to find why my program will compile in Qt 4 but not in Qt 5 after I tried to recreate the .pro file according to what I knew about it at the time. Not an event I'd like to see anyone repeat.

*Since I'm mostly skimming these parts, this may have already been mentioned without me noticing.