QObject::sender() in a Q_PRIVATE_SLOT

[vfernandez]

I have a private class with a slot I've implemented using Q_PRIVATE_SLOT. I need to know which object emitted the signal that is connected to the slot. Since MyClass is not a QObject and the slot is not really implemented in MyClass but in MyClassPrivate, is it safe to use q->sender()?

class MyClassPrivate
{
public:
    ...
    void _q_mySlot()
    {
        Q_Q(MyClass);
        qDebug() << q->sender();
        // should print "foo(0x.....)"
    };
 
    MyClass * const q_ptr;
    Q_DECLARE_PUBLIC(MyClass);
};
 
class MyClass
{
    Q_OBJECT
 
public:
    MyClass(QObject *parent)
    : QObject(parent),
      d_ptr(new MyClassPrivate(this))
    {
        connect(foo, SIGNAL(bar()),
                this, SLOT(_q_mySlot()));
    };
 
    Q_DECLARE_PRIVATE(MyClass);
    Q_PRIVATE_SLOT(d_func(), void _q_mySlot());
    MyClassPrivate * const d_ptr;
};

class MyClassPrivate
{
public:
    ...
    void _q_mySlot()
    {
        Q_Q(MyClass);
        qDebug() << q->sender();
        // should print "foo(0x.....)"
    };

    MyClass * const q_ptr;
    Q_DECLARE_PUBLIC(MyClass);
};

class MyClass
{
    Q_OBJECT

public:
    MyClass(QObject *parent)
    : QObject(parent),
      d_ptr(new MyClassPrivate(this))
    {
        connect(foo, SIGNAL(bar()),
                this, SLOT(_q_mySlot()));
    };

    Q_DECLARE_PRIVATE(MyClass);
    Q_PRIVATE_SLOT(d_func(), void _q_mySlot());
    MyClassPrivate * const d_ptr;
};

To copy to clipboard, switch view to plain text mode 

[vfernandez]

Ok, I answer to myself. I have found plenty of q->sender() calls in the Qt source code, so I guess it's safe.

[squidge]

To see which object emitted the signal, you should really use QSignalMapper, not sender()

[wysota]

Of couse QSignalMapper itself uses sender(), so...

[squidge]

It does, but it's more OO doing it that way, plus it's typically easier to use a simple integer rather than comparing pointers. For example, in a switch statement. The pointer would be non-const, where as the integer would be.

[wysota]

It does, but it's more OO doing it that way, plus it's typically easier to use a simple integer rather than comparing pointers. For example, in a switch statement. The pointer would be non-const, where as the integer would be.

What about eventFilter()? There you have to compare pointers, I don't really see the difference...

QSignalMapper is comparing pointers too so I don't see why one shouldn't have a map similar to the one in the signal mapper directly in his own class, it's always a couple of function calls less... And of course there is a question how to use the signal mapper if the signal we want to use already has some arguments (say... QFtp::commandFinished(int, bool)).

[squidge]

Sure, there are lots of classes in Qt that you can redesign yourself and place into your own code to save a couple of function calls, but why would you want to reinvent such functionality when it is already written? If the current class does not support some functionality (such as multiple arguments) then sure, make a better version, but otherwise, unless a profiling tool is telling you that the class is a bottleneck, then I see no reason not to use it.

[wysota]

Sure, there are lots of classes in Qt that you can redesign yourself and place into your own code to save a couple of function calls, but why would you want to reinvent such functionality when it is already written? If the current class does not support some functionality (such as multiple arguments) then sure, make a better version, but otherwise, unless a profiling tool is telling you that the class is a bottleneck, then I see no reason not to use it.

Here is the situation - you can either use a single function call from a ready API or add a new object that does some stuff and part of that stuff you are actually interested in is calling the same function. Hmm... tough decision. It's like you wanted to hold a boolean value and used QString for that stating that QString was more object oriented and self-contained than bool.

[vfernandez]

Well, since we're talking about a slot in a private class, I'll go the q->sender() way. This makes my implementation really simple and easy to understand. The benefit of QSignalMapper doesn't make up for the overhead I'll get if I use it. Thanks for the suggestion anyway.

[squidge]

Here is the situation - you can either use a single function call from a ready API or add a new object that does some stuff and part of that stuff you are actually interested in is calling the same function. Hmm... tough decision. It's like you wanted to hold a boolean value and used QString for that stating that QString was more object oriented and self-contained than bool.

So what your saying is that QSignalMapper is a waste of processor usage and people should always just use sender() instead? In which case, why was the QSignalMapper class created if it just bloats your code?

[wysota]

So what your saying is that QSignalMapper is a waste of processor usage and people should always just use sender() instead?

No.

In which case, why was the QSignalMapper class created if it just bloats your code?

It's useful if you are waiting for signals from more than one specific object.

[squidge]

Ok, now I'm confused. The OP wanted to know which object emitted the signal that is connected to a particular slot. Assuming the slot had no arguments, why would you use sender() instead of QSignalMapper?

[wysota]

Ok, now I'm confused. The OP wanted to know which object emitted the signal that is connected to a particular slot. Assuming the slot had no arguments, why would you use sender() instead of QSignalMapper?

Here is an example:

QTcpSocket *socket = ...;
QBuffer *buffer = ...;
connect(socket, SIGNAL(readyRead()), this, SLOT(readData()));
connect(buffer, SIGNAL(readyRead()), this, SLOT(readData()));
 
void SomeClass::readData() {
  QIODevice *dev = qobject_cast<QIODevice*>(sender());
  if(!dev) return;
  qDebug() << dev->readAll();
}

QTcpSocket *socket = ...;
QBuffer *buffer = ...;
connect(socket, SIGNAL(readyRead()), this, SLOT(readData()));
connect(buffer, SIGNAL(readyRead()), this, SLOT(readData()));

void SomeClass::readData() {
  QIODevice *dev = qobject_cast<QIODevice*>(sender());
  if(!dev) return;
  qDebug() << dev->readAll();
}

To copy to clipboard, switch view to plain text mode 

You want to know which object emitted the signal because you want to access it and using QSignalMapper for this would be a bit silly.

[squidge]

Ah, now everything makes sense. Indeed, it would be silly to use QSignalMapper and then use the result to index into an array to get a pointer which you had in the first place!

I didn't realise this is what the OP wanted to do, which is why I suggested QSignalMapper. I failed reading the first post.