3
votes

At this point I'm in a dilemma to when to emit a signal vs calling a method in another class directly (same thread). For example, in the tutorial I'm doing I'm connecting the NotifyConnected signal of the Instrument class (Model) to the onConnected slot of 'this' aka The View Manager, refer to SetupViewManager::WireButtons(), third line in code. (I'm using MVVM design pattern). Here signals and slots makes sense as the Instruments class (Model) should not know anything about the View Manager. (i.e. Passing a reference of the view manager to the model is a no no as it would break the MVVM design pattern.) Brilliant.

The issue I have is that next in the tutorial the onConnected slot of the ViewManager then emits other signals which then I have to proceed to manually connect to the slots of another View class i.e. SetupTab (ref void SetupViewManager::onConnected and void SetupViewManager::WireDisplayUpdate() in code).

My question is, why not just replace all the emits in the onConnected slot with calling the methods directly of SetupTab? Feels like over-complicating code to me.

What is the advantage of going the extra mile to emit signals and having to wire up everything just to simply call a public function (signal) from another class which i have a reference for? It's not a multi-threaded application (I know signals and slots are thread safe).

Please enlighten me.

Thanks.

setupviewmanager.cpp:

#include "setupviewmanager.h"
#include "View/setuptab.h"
#include "Model/instrument.h"
#include "Model/settings.h"
#include "utils.h"

namespace Ps
{
    SetupViewManager::SetupViewManager(QObject *parent,
                                       SetupTab &tab,
                                       Instrument &inst,
                                       Settings &config) :
        QObject(parent),
        m_setupTab(tab),
        m_instrument(inst)
    {
        WireSettings(config);
        config.ParseJsonData();
        WireHostAndPort();
        WireMessages();
        WireButtons();
        WireDisplayUpdate();

        m_setupTab.SetHostName(config.getHostName());
        m_setupTab.SetPort(config.getPortNumber());
        m_setupTab.SetCommands(config.getCommandsAsModel());
        auto long_wait = config.getLongWaitMs();
        auto short_wait = config.getShortWaitMs();
        m_instrument.SetlongWaitMs(long_wait);
        m_instrument.SetShortWaitMs(short_wait);
        emit NotifyStatusUpdated(tr("Long wait Ms: %1").arg(long_wait));
        emit NotifyStatusUpdated(tr("Short Wait Ms: %1").arg(short_wait));
        onDisconnected();
    }

    SetupViewManager::~SetupViewManager()
    {
        Utils::DestructorMsg(this);
    }

    void SetupViewManager::WireSettings(Settings &config)
    {
        connect(&config, &Settings::NotifyStatusMessage, &m_setupTab, &SetupTab::onStatusUpdated);
    }

    void SetupViewManager::WireHostAndPort()
    {
        connect(&m_setupTab, &SetupTab::NotifyHostNameChanged, &m_instrument, &Instrument::onHostNameChanged);
        connect(&m_setupTab, &SetupTab::NotifyPortChanged, &m_instrument, &Instrument::onPortChanged);
    }

    void SetupViewManager::WireMessages()
    {
        connect(&m_instrument, &Instrument::NotifyErrorDetected, &m_setupTab, &SetupTab::onStatusUpdated);
        connect(&m_instrument, &Instrument::NotifyStatusUpdated, &m_setupTab, &SetupTab::onStatusUpdated);
        connect(this, &SetupViewManager::NotifyStatusUpdated, &m_setupTab, &SetupTab::onStatusUpdated);
    }

    void SetupViewManager::WireButtons()
    {
        connect(&m_setupTab, &SetupTab::NotifyConnectClicked,&m_instrument, &Instrument::Connect);
        connect(&m_instrument, &Instrument::NotifyConnected, &m_setupTab, &SetupTab::onConnected);
        connect(&m_instrument, &Instrument::NotifyConnected, this, &SetupViewManager::onConnected);

        connect(&m_setupTab, &SetupTab::NotifyDisconnectClicked,&m_instrument, &Instrument::Disconnect);
        connect(&m_instrument, &Instrument::NotifyDisconnected, &m_setupTab,&SetupTab::onDisconnected);
        connect(&m_instrument, &Instrument::NotifyDisconnected, this, &SetupViewManager::onDisconnected);

        connect(&m_setupTab, &SetupTab::NotifySendClicked,&m_instrument, &Instrument::onSendRequest);
        connect(&m_instrument, &Instrument::NotifyDataSent,&m_setupTab, &SetupTab::onDataSent);

        connect(&m_setupTab, &SetupTab::NotifyReceiveClicked,&m_instrument, &Instrument::onReceiveRequest);
        connect(&m_instrument, &Instrument::NotifyDataReceived,&m_setupTab, &SetupTab::onDataReceived);
    }

    void SetupViewManager::WireDisplayUpdate()
    {
       connect (this, &SetupViewManager::NotifyConnectEnabled, &m_setupTab, &SetupTab::onConnectEnabled);
       connect (this, &SetupViewManager::NotifyDisconnectEnabled, &m_setupTab, &SetupTab::onDisconnectEnabled);
       connect (this, &SetupViewManager::NotifyDirectCommandsEnabled, &m_setupTab, &SetupTab::onDirectCommandsEnabled);
       connect (this, &SetupViewManager::NotifyControlTabEnabled, &m_setupTab, &SetupTab::onControlTabEnabled);
    }

    void SetupViewManager::onConnected()
    {
        emit NotifyConnectEnabled(false); // HERE. Why not just call method directly with m_setupTab.onConnectEnabled(false); etc...?
        emit NotifyDisconnectEnabled(true);
        emit NotifyDirectCommandsEnabled(true);
        emit NotifyControlTabEnabled(true);
    }

    void SetupViewManager::onDisconnected()
    {
        emit NotifyConnectEnabled(true);
        emit NotifyDisconnectEnabled(false);
        emit NotifyDirectCommandsEnabled(false);
        emit NotifyControlTabEnabled(false);
    }
}
2
Can you change your text format, so it is easier for the eye to read? Also, a piece of code might clarify the question some more.JefGli
Do you set a view to a ViewManger? Shouldn't you make these connections there? It's very difficult to tell when we don't know anything about these classes.thuga
Done as requested. Kindly refer to code. Thank you!Nokiaowner
Well in this case it hardly makes any difference. You can connect to these signals from the outside, but if you don't intend that to happen, maybe it is better to call the methods of m_setupTab directly.thuga
So what you're saying is, it would be correct to call the methods in Setuptab directly in SetupViewManager::onConnected() with m_setupTab.functionName(param) rather then emitting a signal and then connecting the signals/slots. Correct?Nokiaowner

2 Answers

2
votes

Advantages of signal-slot mechanism:

  • easy to use when your class has no information about it's clients;
  • may be used for thread-safe calls;
  • you must not manually remember all objects to notify them;
  • the only rule to connect two objects is that they both must be QObject subclasses.

Disadvantages:

  • slower call (each signal emit scans list of all connected objects);
  • possibly complicated spaghetti-code; you don't know, who and when will call any slot or who will get emitted signal.

You should think yourself about your case. If there is no signal "listeners" outside the SetupViewManager, try direct calls. If someone else can connect to this signals, your choice is emitting them.

There also may be other reasons to use signals. But there is no reason to use them just to call a function. In one thread, at least.

2
votes

Signals and slots are used to decouple classes so that they don't need to explicitly know who uses their functionality and how. In many cases, decoupling is a desirable trait of the design of your software. Of course it's not an end in itself, it's useful when it helps you to reason about the code's correctness and makes it easier to maintain. Decoupling help in understanding/reasoning about the code as it leads to smaller units of code that you can analyze in isolation. Another way to look at it is separation of concerns: let one unit of code do one thing, e.g. focus one class on one aspect of functionality.

When you have a pair of classes and wish to decide whether to couple them or not, think of whether they could be used with other classes. A could be coupled to B, but could the interface that couples the pair be used by C instead of B? If so, then some decoupling pattern must be used, and signal-slot pattern is one of them.

For example, let's compare how these two interfaces affect coupling with user code. The objective is simple: add debug output to an object's destructor:

class QObject {
  ...
  Q_SIGNAL void destroyed(QObject * obj = Q_NULLPTR);
};

class QObjectB {
  ...
  virtual void on_destroyed();
};

int main() {
  QObject a;
  struct ObjectB : QObjectB {
    void on_destroyed() override { qDebug() << "~QObjectB"; }
  } b;
  QObject::connect(&a, &QObject::on_destroyed, []{ qDebug() << "~QObject"; });
}

The signal-slot interface allows you to easily add functionality to existing objects without a need to subclass them. It is a particularly flexible implementation of the Observer pattern. This decouples your code from the code of the objects.

The second implementation, using the template method lookalike pattern, forces a closer coupling: to act on ObjectB's destruction, you must have an instance of a derived class where you implement the desired functionality.