[Qt-interest] connect()ing multiple buttons to a receiver
Tom Panning
lurchvt at gmail.com
Mon Jan 12 18:07:44 CET 2009
I'll try to clarify what I think Oliver meant by modularity. The
important point with regards to QObject::sender() versus QSignalMapper
is not the code that is inside the slot, but the interface that the
slot presents to the rest of the program. If you use
QObject::sender(), then (in the simple case) the slot takes no
parameters but must be called through Qt's signal-slot mechanism by
the object that the slot needs a pointer to. If you use QSignalMapper,
then the slot takes one argument, which is a pointer to the object
that the slot needs.
Let me try an (admittedly contrived) example. I want to print the text
of a QPushButton when it is pushed. So using QObject::sender() I would
do this:
// Slot that prints button text.
void MyClass::printButtonText()
{
QObject* sender = QObject::sender();
if (sender != NULL) {
QAbstractButton* button = dynamic_cast<QAbstractButton*>(sender);
if (button != NULL) {
qDebug() << button->text();
}
}
}
// Create and connect the buttons
void MyClass::createButtons()
{
QVector<QPushButton*> buttons(10);
for (int i = 0; i < 10; i++) {
buttons[i] = new QPushButton(QString::number(i)); // There would
be real text here.
connect(buttons[i], SIGNAL(clicked()), this, SLOT(printButtonText()));
}
}
To use QSignalMapper, it would look like this:
// Slot that prints button text.
void MyClass::printButtonText(QWidget* sender)
{
if (sender != NULL) {
QAbstractButton* button = dynamic_cast<QAbstractButton*>(sender);
if (button != NULL) {
qDebug() << button->text();
}
}
}
// Create and connect the buttons
void MyClass::createButtons()
{
QVector<QPushButton*> buttons(10);
QSignalMapper* mapper = new QSignalMapper(this);
for (int i = 0; i < buttons.size(); i++) {
buttons[i] = new QPushButton(QString::number(i)); // There would
be real text here.
mapper->setMapping(buttons[i], buttons[i]);
connect(buttons[i], SIGNAL(clicked()), mapper, SLOT(map()));
}
connect(mapper, SIGNAL(mapped(QWidget*)), this,
SLOT(printButtonText(QWidgetI)));
}
Now at this point, it is obvious that the innards of the slot are
virtually identical between the two cases. It is also obvious that
adding the QSignalMapper requires more work at construction time.
However, what happens if I want to call printButtonText() on each
button? This is where the modularity comes in. The slot in the
QSignalMapper example can easily be called as a method because it
doesn't depend on anything other than its arguments, so I can just do:
for (int i = 0; i < buttons.size(); i++) {
printButtonText(buttons[i]);
}
But I can't do anything like that for the QObject::sender() example.
My only recourse is to call QAbstractButton::click() on each button to
force the clicked() signal to be emitted. But, that seems rather messy
(what if the signal is connected to other slots?), and not all signals
have a corresponding method that causes them to be emitted. (If you
were wondering, this is why there is a QSignalMapper::map(QObject*)
slot.)
And besides that, the QObject::sender() slot isn't intuitive to use.
>From the method signature, there is no hint that it's expecting to get
information through the signal-slot mechanism. Of course, you can add
a comment that explains it. But if you've ever worked on a large
programming project, you know that comments aren't always read (or
maintained, or even written). But the method signature will always be
maintained because otherwise the code doesn't compile.
I'll make one last attempt to explain why QObject::sender() should be
treated with a light and careful touch. C++ doesn't have a standard
way to examine the call stack at run-time, but some interpreted
languages do. In those languages it is considered risky to make a
function change its behavior dependent on the method that called it,
which is exactly what QObject::sender() does.
I believe the above example completely explains why I think
QSignalMapper should be used instead of QObject::sender() whenever
possible. If you still disagree then to keep this thread from getting
completely out of hand, I think we will just have to agree to
disagree.
Tom
P.S. I didn't attempt to compile the example code, so I make no
guarantees that it works.
On Fri, Jan 9, 2009 at 10:27 PM, Malyushytsky, Alex <alex at wai.com> wrote:
> 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."
>
> _______________________________________________
> Qt-interest mailing list
> Qt-interest at trolltech.com
> http://lists.trolltech.com/mailman/listinfo/qt-interest
>
More information about the Qt-interest-old
mailing list