[Qt-interest] connect()ing multiple buttons to a receiver

[Malyushytsky, Alex]

Oliver,

Thank you for your valuable comments, but I afraid you are missing the point.

The only difference in the code when using QSignalMapper and sender() in that "special case" we were discussing, is the way you obtain a pointer to QObject in the slot which peforms actions (either through calling sender() in this slot or as a parameter added by QSignalMapper).

The way this pointer is handled after (and that is the code you are trying to argue about: "dirty", "unsafe", etc) is exactly the same in both cases,
"(with all the whistles and bells, such as dynamic casting and a big fat ASSERT(FALSE),".

What I say is, that usage of sender() may produces at least as good code, as QSignalMapper. In some situation this code will be better and preferable even, if you could use QSignalMapper (opposite to what you said).

The reason to use or not to use QSignalMapper is not that it "hides" call to sender(). Mapping is what makes the difference.

QSignalMapper adds mapping (integer ID, QString, to QObject*,) to signal handling. This is great and convenient, if you would like to use mapping in your code -  read "You would still do mapping yourself, if QSignalMapper did not exist".
It does hide usage of sender(), because it is class implementation.
But purpose of the class is to provide user mapping, which might allow to write better code, and not in hiding the call to the sender() alone.

In fact the following class which only hides the call to sender() and does not provide mapping would be useless, but if follow your arguments perfect:

class SignalMapper : public QObject
{
    Q_OBJECT
public:
    explicit QSignalMapper(QObject *parent = 0);
    ~QSignalMapper();

Q_SIGNALS:
    void mapped(QObject *);

public Q_SLOTS:
    void map() {
      QObject* s = sender();
      if (s)
        emitt mapped( s );
    }
}

Because of its design, QSignalMapper should be used over sender() when you can take advantage from the mapping ( to integer, string, QWidget* or even other QObject*) it provides and can afford it.
It is certainly not the case we were discussing:
array of the pointers and action required depends on the QObject properties: type, class or object name or even pointer value itself.

Mapping comes with its own cost, memory, time of execution and development, and even complexity of the code. If you don't need it, stay away from the QSignalMapper.

Cost is small in some cases?
Yes, if you have 5 senders it is small, but even in this case it might be considered a bad style, cause it does not provide any advantages, slows down the development (you agreed that using sender() is fast) and increases complexity of the code.
Would you really like to maintain any useless set of data (hash table in our case), if you have a choice?

All said above was about usage of QSignalMapper over QObject::sender().
"Modularity, Modularity, Modularity!" is irrelevent and is not a reason to make a decision in such case.
QSignalMapper is a convenience class which you should use when it is convenient and affordable.
You could write similar to QSignalMapper class which would be able to map signals lets say with QString or integer parameters ( well if you don't afraid using sender() ).
At the same time avoiding sender() when you don't need mapping is unreasonable.

QButtonGroup, QActionGroup, QSignalMapper, QObject::sender(), ... name it, has its own usage.
Yes, if you have SMALL, LIMITED number of push buttons in a dialog, you don't want to use sender() nor QSignalMapper. For SMALL CONSTANT number of buttons (less than 10?) I would not even use (none-exclusive) QButtonGroup.
In such case I believe creating a slot for each button is better design. The same could be said about (none-exclusive) QActionGroup.

Best regards,
       Alex


-----Original Message-----
From: qt-interest-bounces at trolltech.com [mailto:qt-interest-bounces at trolltech.com] On Behalf Of Oliver.Knoll at comit.ch
Sent: Friday, January 09, 2009 12:51 AM
To: qt-interest at trolltech.com
Subject: Re: [Qt-interest] connect()ing multiple buttons to a receiver

Malyushytsky, Alex wrote on Friday, January 09, 2009 5:30 AM:

>>> Imagine what would happen if anyone would connect any *other*
>>> object's signal (other than from a QButton!) to your slot: Your code
>>> above would
>
> Referenced code has not being meant even to be a perfect C++

I wasn't refering to the "code quality" of your example code which was perfectly fine for showing the usage of sender(). It is perfectly accetable to write "pseudo code" (and omit error checking etc.) in order to show the *basic* idea.

I was objecting to the usage of sender(), in the same way the Qt documentation does, see below. Well, not so much in the sense "never ever use it! Never ever!", but more in the sense "Try to avoid it, and especially be aware of the consequences IF you use it!".

>  Irrelevant to
> the question asked or situational checks were omitted to make answer
> easier to understand.

I totally agree.

> The purpose of the code was to demonstrate how to locate which object
> sent a signal, cause person who asked the question said : "I know
> about QObject::sender(), but I still don't know of how to tell
> *which* button it points to at that point." That what it did I
> believe.

And I didn't object to that ;)

>
>>> Again: for quick'n'dirty code or code which is used just very
>>> "locally" in a large application it might be okay to use sender().
>>> But I would refrain from using sender() if the slot was public or
>>> protected and you could never foretell what other code will call it
>>> (maybe in 2 years away from now...).
>
> I disagree that usage of sender() is "dirty" or unsafe. It does not
> matter, if slot is protected, public or private.

Yes, as someone else already mentioned the "visibility" of methods is *not* taken into account when connecting signals with slots.

But in fact this makes my argument even more valid! ;)

> Any code is as safe, as you make it.

Well, this is as true as the following expression: true == true

> All arguments mentioned about unsafe usage of sender() are related to
> the safety of pointer usage. If you deal with C++, you deal with
> pointers and have to understand how to make their usage safe.

WRONG! I did NOT argue about pointer safety! :) I am talking on a much "higher level", not related to any programming language features.

I am talking about "modularity" and "dependency" of your code. I will explain it by commenting on the example you gave below.

> ...
> 2) If you have pointer to QObject, use inherits() or qobject_cast
>
> QObject* s= sender();
> if ( s )
> {
>    if (s ->inherits("QComboBox") )   {  .....
>    }
>    else if (s->inherits("QAbstractButton")   {  .....
>    }
>    else
>      Q_ASSERT(false);
> }
> Usage inherits() not only provides the safety check, but also allows
> slot to handle different clases as a sender.

While this code perfectly works AS LONG AS YOU ONLY CONNECT QComboBox and QAbstractButton signals to that slots, it miserably fails for all other (not in the inheritance hierarchy of the given objects) objects!

And what's even worse: you make your slot DEPENDENT on QComboBox and QAbstractButton, that is the slot has to "know" about the existence of such widgets. This violates the principle of modularity (hint hint: I already used this word twice, so it must be important - even the Qt docs mention it ;)

And on the same level of argument you also use "code locality", that is if you add a new GUI element which is to be connected to these slots you have to CHANGE THE IMPLEMENTATION of these slots as well (by adding more "if inherits"-clauses)! Imagine what that would mean if the GUI elements where to be connected to several slots, all distributed over your code. A maintenance nightmare.

Modularity, Modularity, Modularity!

> Conclusion:
> QT itself is using sender() and there is nothing wrong for anybody
> else to use it.

In fact, as someone else mentioned it, QSignalMapper is using it. But that's perfectly fine, because I don't care what's under the hood of Qt (and believe me, I took a look several times under the hood of Qt, and once I had to parse e.g. TrueType font info myself - I am SOOOO glad that "they" do the dirty work and give us a clean and easy API - such as QSignalMapper ;)


> It is a part of the official API and is as safe as
> using pointers in general.

<sarcasm>I never trust statements which end with "in general" - I have learned that as a programmer you mostly deal with "special cases" ;)</sarcasm>

>   Even referenced QT4 documentation says:
>>> "However, getting access to the sender might be useful when many
>>> signals are connected to a single slot."

As I stated in a previous post of mine this is also the only useful scenario where you would actually consider using sender() anyway, so this sentence doesn't really give you much more info  ;)

>
> I don't want to push anybody to a specific decision. I would say that
> using sender() in my cases leaded to the simpler and faster code, but
> I might chose different solution in other case.

I doubt that the performance penalty - if there is any - is noticeable in a GUI application, when comparing sender() with e.g. QSignalMapper. And in doubt I go for code readability and maintainability, willing to easily sacrifice say 10% of performance in such an area (and I am not talking about high performance code such as 3D, number crunching or similar applications).

> I would recommend do not make decisions based on incorrect arguments.

I hope I could convince you know that my argument is valid ;)

> Also QSignalMapper is limited to parameterless signals, so it is good
> to know about sender() anyway.

Good point. But luckily in practise this is rarely a problem: if you connect multiple QActions to a slot you would use QActionGroup and the signal triggered() provides you with the original QAction which has been clicked - no need to call sender(). For push buttons you have the QButtonGroup which works in analogy. Again no need to call sender(). This covers about all cases I have ever needed to use (for a Recent Files menu, for example, or a radio button group).


Again, I didn't want to say "Never ever use it". It is fine for code which is "closed in itself", e.g. a Recent Files menu where admittedly it is very unlikely that someone would connect signals to the slot other than from the "Recent File menu", so calling sender() in the "openRecentFile()" (with all the whistles and bells, such as dynamic casting and a big fat ASSERT(FALSE), just as you have shown above) is probably okay.

And once you know the implications of using sender it is "safe" (in your words) to use it, I agree.


Cheers, Oliver
--
Oliver Knoll
Dipl. Informatik-Ing. ETH
COMIT AG - ++41 79 520 95 22

_______________________________________________
Qt-interest mailing list
Qt-interest at trolltech.com
http://lists.trolltech.com/mailman/listinfo/qt-interest


---------------------------------------------------------------------------------------------------
Weidlinger Associates, Inc. made the following annotations.

"This message and any attachments are solely for the intended recipient and may contain confidential or privileged information. If you are not the intended recipient, any disclosure, copying, use, or distribution of the information included in this message and any attachments is prohibited. If you have received this communication in error, please notify us by reply e-mail and immediately and permanently delete this message and any attachments. Thank you."

"Please consider our environment before printing this email."