Reasons for signal/slot connection with sender == receiver?

[Bart_Vandewoestyne]

While reviewing some legacy code, I came across the following:

connect(this, SIGNAL(fooSignal(int)), this, SLOT(fooSlot(int)));

My initial thought was that this was not useful, as the

emit fooSignal(x);

can simply be replaced by

fooSlot(x);

However, I see one reason for not replacing the emit by the slot, and that is if the signal is connected to multiple different signals. In that case, the replacement cannot be made.

But in the code I'm reviewing, fooSignal is only connected to fooSlot, so my educated guess would be that this connection is useless and I can replace the emit by the call to fooSlot.

Am I right or am I overlooking things? In case there is only one slot connected, are there other reasons for keeping this signal/slot connection instead of simply replacing it by a call to fooSlot?

[VRonin]

1. if you replace SIGNAL/SLOT with Qt5 connection ther's no real performance impact in the connection.
2. signals are (almost) always public so fooSignal signal that you just use to invoke fooSlot can be use by somebody else for something.
3. if the same signal is connected to multiple internal slot it might be just a way to simplify mutiple calls

[Pablo J. Rogina]

@Bart_Vandewoestyne said in Reasons for signal/slot connection with sender == receiver?:

But in the code I'm reviewing, fooSignal is only connected to fooSlot

Do you mean in the same class or in the whole project?

[kshegunov]

To add one more to @VRonin's list, or rather to envelop his third point into more general reasoning:
4) Using it that way decouples the components - i.e. you have clear separation between the event that has occurred (the signal) from the code that responds to it (i.e. the signal/slot).

[Bart_Vandewoestyne]

@Pablo-J.-Rogina said in Reasons for signal/slot connection with sender == receiver?:

@Bart_Vandewoestyne said in Reasons for signal/slot connection with sender == receiver?:

But in the code I'm reviewing, fooSignal is only connected to fooSlot

Do you mean in the same class or in the whole project?

The signal fooSignal is only connected to fooSlot and both this signal and slot are from the same class. The whole project has no other slots connected to fooSignal.

From the other answers in this thread, I would conclude that I can safely replace the emit of fooSignal by a call to fooSlot (thereby improving the readability of our code).

[Pablo J. Rogina]

@Bart_Vandewoestyne ok, if your issue is solved please don't forget to mark your post as such. Thanks.

[Bart_Vandewoestyne]

I had made the change, but our unit tests started failing because of this change and the conclusion was that I overlooked something. For more details, see my other post https://forum.qt.io/topic/89103/qtcpsocket-read-reads-0-bytes-and-it-shouldn-t-why

In summary: replacing the signal/slot connection by a direct call, resulted in a QTcpSocket being read from another thread then the thread to which the QTcpSocket belongs to, which resulted in a race condition/undefined behavior.

Main conclusion: also watch out not to introduce race conditions/undefined behavior when replacing a signal slot connection by a direct call!

[J.Hilk]

@Bart_Vandewoestyne
That is actually the reason, why I always pass the 5th parameter - Qt::QueuedConnection - to QObject::connect, when I know, that the connection crosses threads.

Had the legacy code writer done that as well, you would have never thought about replacing that connect with a direct function call.

PS: you can also use QMetaObject::invokeMethod instead of signal slot and connect, it excepts a connectiontype as argument and its shorter if you call the method only once.

[kshegunov]

@J.Hilk said in Reasons for signal/slot connection with sender == receiver?:

That is actually the reason, why I always pass the 5th parameter - Qt::QueuedConnection - to QObject::connect, when I know, that the connection crosses threads.

That assumes you know the call will be across threads. One of the reasons to have AutoConnection is to have the behaviour independent of the receiver's thread. Also by passing Qt::QueuedConnection, you enforce the event to go through the event loop, which isn't necessary in most cases (think also copying of the slot's arguments), and furthermore it may cause out of order execution in your emitting object in some rare cases (i.e. when there's a lot of ping-ponging of signals and slots). So a good rule of thumb is just to use the auto connection for 99% of cases.

[LScott]

I actually use this quite a bit to connect a slot of a derived class to a signal of the base class. True, this could be done with pure C++, but in some instances I find it more convenient and readable. E.g:

class BaseClass : public QObject
{
    Q_OBJECT

    Q_PROPERTY(QColor colour READ GetColour WRITE SetColour NOTIFY ColourChanged FINAL)

    public:
        BaseClass(QObject *parent = NULL) : QObject(parent) { _colour = QColor("#000000");}

        QColor GetColour() const { return _colour; }
        void SetColour(const QColor &c) { _colour = c; emit ColourChanged(); }

    signals:
        void ColourChanged();

    private:
        QColor _colour;
};

class DerivedClass : public BaseClass
{
    Q_OBJECT

    Q_PROPERTY(QString colourString READ GetColourString NOTIFY ColourStringChanged)

    public:
        DerivedClass(QObject *parent = NULL) : BaseClass(parent)
        {
            connect(this &BaseClass::ColourChanged, this DerivedClass::ColourStringChanged);
        }

        QString GetColourString() const { return GetColour().toString(); }
    
    signals:
        void ColourStringChanged();
    
};

Yes, I realize I'm connecting a signal to another signal, instead of a slot, but the exact same thing applies. For example, if you wanted to store the colour string internally for some reason (this is probably a bad example, as there are better ways to do that, but there are plenty of scenarios when this comes in handy, because the only other way to do the same thing is kind of clunky) you could do this:

(DerivedClass)
public:
    DerivedClass(QObject *parent = NULL) : BaseClass(parent)
    {
           connect(this &BaseClass::ColourChanged, this DerivedClass::UpdateColourString);
    }

    QString GetColourString() const { return _colourString; }

public slots:
    UpdateColourString()
    {
        _colourString = GetColour().toString();
        emit ColourStringChanged();
    }