Table of Contents

Rosegarden Coding Style: Qt Layouts

When doing new work in Rosegarden, it is perfectly acceptable to use Qt Designer if you prefer. One good example of a new Designer-based dialog is the MIDI device manager. However, most of our layout code is hand-written Qt, and most of us have seemed to prefer this approach over the years, so we will probably go on having a high proportion of hand-written layout code.

Much of our existing layout code started when KDE and Qt were at version 2.x, and it has survived the transition to Qt and KDE 3.x and then finally to Qt 4.x. Adding to the amount of legacy cruft present in this code, much of the transition to Qt 4 was done by automatic conversion scripts. This has left us with a lot of layout code that gets the job done, but it is a terrible mess, and full of all kinds of inconsistencies.

After doing a lot of work on these legacy layouts, I think it is time to set out some guidelines for new work so that people will be encouraged to follow best practices going forward, and not look back at bad old examples. Some of the strange things I've been dealing with have been from scripts that could only do so much automated work, and some it is actually from the initial conversion guidelines that were published at the start of the porting effort, which recommended practices that seem bizarre and confusing to me today, but probably seemed like sensible best guesses to make at a time when all of this was months away from compiling, and there was no way to get immediate feedback and try different ideas.

A dialog box IS a QWidget

It is true that in order to add a layout in Qt 4 you have to have a QWidget, but if you have a QDialog (for example), you have a QWidget. You do not need to create a new QWidget inside the dialog and add a layout to that.

This code creates a layout of type QGridLayout and sets it to be the layout for the QDialog object, then it creates a new QWidget called vBox, and sets up a QVBox layout inside it. This extra complication is not necessary, and it should be avoided in future code. We can reduce all of this to:

All of this vBox begat frame begat QPushButton code can be removed for simplicity. (QLayout::addItem() which is eventually called by addWidget() will take ownership of the widget. So there is no danger of memory leaks.) It is rarely necessary or beneficial to supply all these optional parent parameters. This has many practical advantages, and has been greatly helpful to me in resolving broken layouts during this restructuring work. If an existing layout ain't broke, there is little point in fixing it, but please do not write code like this in new layouts. The example above will work perfectly fine when reduced to:

The QDialogButtonBox is created with three parameters, and then the object's name is set with a follow-up call to setObjectName() that looks like it is probably the result of automatic conversion, as none of the modern ctors for this class take a name parameter:

We have never done much with the name property, and it is just as well whenever it happens to have been split out of a declaration like this one. Most objects we have named throughout our code have names that serve no particular purpose. Of the few exceptions to this rule of thumb, almost all meaningful object names are related to our new stylesheet-based look.

I suggest using the excellent ack utility to search the code tree for occurrences of any questionable name. In this case, it looks like “dialog_base_button_box” used to be named for some purpose that is probably no longer valid:

We can and should remove the parent entirely, and we could use setStandardButtons() and setOrientation(). It is not such a big deal to refactor changes like these last two in current code, but it is a good idea to use this kind of idiom in the future. The entire code snippet, then, could be written out like this:

It is true that this requires more typing, but it saves having to browse Qt documentation to find out what the parameters are when revisiting old code, and it doesn't take code very long at all to become old in terms of remembering what does what. Just in the six or so months I've been most deeply involved with all this layout work, I've really learned to appreciate the examples where I did the kind of refactoring I'm promoting as the good practice standard here.

You Can Set A Layout Before You Populate It!

This kind of thing is driving me closer and closer to going insane the more often I deal with it.

Somebody promoted the practice of calling setLayout() on a widget only after populating that layout with widgets. We have widget creation here, and then 100 lines down, the widget gets a layout, usually in code where some other widget was just created (which widget won't get its own layout until much further down in turn.)

I have some vague idea that someone believed you couldn't set a layout on something until the layout was populated. That is not true at all. Go ahead and set the layout immediately, then populate it later.

Here is an unusually simple and compact example to follow, with extra comments added: