jump to navigation

How to (not) Activate or Deactivate Widgets (or any other thing) October 29, 2015

Posted by tumaix in planetkde-tomazcanabrava.
trackback

Looking at the source code of a few programs while I was hunting for things to do I found similar patterns to this one:

	QAction *action = new QAction("Action Name Here", this);
	connect(action, &QAction::triggered, this, &MainWindow::viewActionTriggered);
	actionList << action; 
        for(QAction *action : actionList) { 
             mapWidgetAction[action->text()] = createWidget(action->text());
	}
	MainWindow::viewActionTriggered() {
		QSettings settings;
		QAction *action = qobject_cast<QAction*>(sender());
		if (action->text() == "Widget 1") {
			widget1->show();

		} else if (action->text() == "Widget 2") {
			widget2->show()
		} else if (action->text() == "Widget 3") {
			widget3->show();
		}
		s.setValue("visibleWidget", action->text());
	}

Now, looking at this particular code it’s simple to see what could be wrong: the Widget Title could be translated to another language and the comparissons would fail, or the text of the widget could have an ‘&’ to create an accelerator and the comparisson would also fail, you could change your language and when opening the application it could also fail, there are actually tons of possible errors with that approach.

One of the possibilities is to use the data() member of QAction to store a untranslated string, and use that as comparator:

	QAction *action = new QAction("Action Name Here", this);
	action->setData("Widget 1");
	connect(action, &QAction::triggered, this, &MainWindow::viewActionTriggered);
	actionList << action; 
        for(QAction *action : actionList) { 
           mapWidgetAction[action->text()] = createWidget(action->text());
	}
	
	[snip]

	MainWindow::viewActionTriggered() {
		QSettings settings;
		QAction *action = qobject_cast<QAction*>(sender());
		if (action->data().toString() == "Widget 1") {
			widget1->show();

		} else if (action->data().toString() == "Widget 2") {
			widget2->show()
		} else if (action->data().toString() == "Widget 3") {
			widget3->show();
		}
		s.setValue("visibleWidget", action->data().toString());
	}

But that second approach has also tons of flaws: I’m not caching the QString returned, I’m returning a QString while I could have used a QByteArray(much faster for non-utf8 stuff), I could have used something that doesn’t use multiple cpu-cycles to tell me if it’s true or not. Now, third try:

	enum WidgetTypes { WIDGET_1, WIDGET_2... , WIDGET_X };
	QAction *action = new QAction("Action Name Here", this);
	action->setData( WIDGET_1 );
	connect(action, &QAction::triggered, this, &MainWindow::viewActionTriggered);
	actionList << action; 
        for(QAction *action : actionList) { 
             mapWidgetAction[action->text()] = createWidget(action->text());
	}
	
	[snip]

	MainWindow::viewActionTriggered() {
		QSettings settings;
		QAction *action = qobject_cast<QAction*>(sender());
		switch(action->data().toInt()) {
		case WIDGET_1 : widget1->show(); break;
		case WIDGET_2 : widget2->show(); break;
		...
		case WIDGET_X : widget3->show(); break;
		}

		s.setValue("visibleWidget", action->data().toInt());
	}

By changing the key of the Widget to an enum I’m having the following benefits

  1. Not Comparing Strings, but comparing integers (much faster)
  2. Not having issues with collation
  3. simplifying my code (switch vs if’s with string comparissons)
  4. behaving correctly even if I change the system’s language
  5. now, I know that this is something that seems easy enougth to catch, but since I loocked this in three different softwares already this week I think it’s worth saying.

 

Now, if there’s anything wrong in what I said, please correct me as we can only improve by learning something new.🙂

Comments»

1. Matteo Italia - October 29, 2015

Why bother with the map or custom identifiers? Just store the pointer to the QWidget in setData, or directly connect to a lambda created on the spot and let it capture the relevant context.

tumaix - October 29, 2015

how could you store a pointer on the QSettings, close the app, then restore the view when the app is reopened?🙂

Matteo Italia - October 29, 2015

Wops, sorry, I missed the QSettings bit😉

2. IWatchTooMuchAnime - October 29, 2015

Use QSignalMapper.

Don’t write a user visible string to settings. You might need to change the string for whatever reason. Use some of the Widget object’s metadata, like the object name (QObject::objectName.)

tumaix - October 29, 2015

That’s a good approach.

3. anonymous - November 1, 2015

Interesting!

Only note, comparison has one s😉

tumaix - November 1, 2015

the perils of having portuguese as first language…🙂


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: