I've earned experience in C but I'm new at Qt. I was given a real project to work on, developed by someone that doesn't work for this company any longer. I don't know if it is a good practice and I apologize in advance for the terminology that might well not be adedcquate: I noticed that this project is literally full of signals/slots pairs that I deem unnecessary. More precisely: the classes that dictate the logic of the applications see each other, it would be sufficient to expose some public methods to trigger the desired procedures but, nevertheless, this is almost always achieved using signals and slots (and I say it again here: even when no input from GUI occurs). Given that I'm a newby in Qt, is it a good practice to do so? Thanks.

Edit: the cases that I reported don't encompass signals coming from timers, threads or whatever. This guy used signals and slots pairs like if it was a substitution for direct method call from class, say, A to class B

CodePudding user response：

It depends of course, but mostly yes, it's a correct practice because it keeps object decoupled. Two classes which sees each other does not means they can use each other if they're not in a relation of master-slave o don't follow a logical hierarchy. Mostly you will couple everything in a non-reversible way in a result of flipper-effect of calls. The proof could be you want to fix that "making methods public" which may breaks incapsulation and contract of a class, which may lead to bad design choice non dependant from using Qt.

Since we're not seeing the actual code it could be he is misusing signals too, but from your explanation I'd go with the first option.

CodePudding user response：

Overuse of signals and slots is a very bad and unfortunately very common practice. It hides dependencies, it makes code hard to debug and basically unmaintainable in the long term. Unfortunately many programmers think this it is a good practice because they achieve "decoupling", which seems as a holy grail to them. This is a nonsense.

I do not say you should not use signals and slots at all. I only say you should not overuse them. Signals and slots are the perfect tool to implement Observer design pattern to have a "reactive" system in which objects react to other objects having changed their states. Only this is the correct use of signals and slots. Almost every other use of signals and slots is wrong. The most extreme case which I have seen was implementing a getter function with a signal-slot connection. The signal sent a reference to a variable and the slot filled it with a value and then it returned to the emitter. This is just mad!

How you know that your signals and slots implement Observer pattern correctly? These are rules of thumb which follow from my quite long experience with Qt:

1. The nature of the signal is that the emitter announces publicly (signals are always public - except if you use a private class dummy parameter) by sending out some signal that its state has somehow changed.
2. The emitter does not care who are the observers or whether there are any observers at all, i.e. the emitter must not depend on observers in any way.
3. It is never the emitter's responsibility to establish or manage the connection - do not ever do it! The connection/disconnection is the responsibility of the observer (then it often connects to a private slot) or of some parent object which knows of the existence of both, the emitter and the observer (in that case the mutual parent connects emitter's signal to observers public slot).

It is normal that you will see lots of signal-slot connections in GUI layer and this is perfectly OK (note: GUI layer includes view models!). This is because GUI is typically a reactive system where objects react to other objects or to some changes in the underlying layers. But you will probably see much less signal-slot connections in the business logic layer (btw. in many projects business logic is coded without using Qt).

Regarding naming: I have encountered an interesting code smell. When the observer's public (!) slot is called like onSomethingHappened() - with emphasis on the prefix on. This is almost always sign of bad design and abuse of signals and slots. Usually this slot should be a) made private and the connection should be established by the observer or b) should be renamed to doSomething() or c) should be renamed and should be called as normal method instead of using signals and slots.

And a note about why overuse of signals and slots are hard to maintain. There are many potential problems in the long term which can break your code:

1. The dependencies with signals and slots are often hidden in a distant seemingly unrelated part of code. This relates to the signal-slot abuse when emitter actually depends on the observer but this is not clear when looking at the emitter's code. If your class depends on some other class/module, this dependency should be explicit and clearly visible.
2. When signals and slots are connected and then disconnected programmatically by your code, you often end up in state when you forgot do disconnect and you now have multiple connections. Having multiple connections is often overlooked because it often does not do any harm, it only makes the code somewhat slower, i.e. changed text is updated multiple times instead of once only - nobody will catch this issue unless you have a thousand-fold connection. these multiplying connections are somewhat similar to memory leaks. Small memory leaks remain often unnoticed, which is similar to multiple connections.
3. It often happens that you depend on the order in which the connections are established. And when these order-dependent connections are established in distant parts of code, you are in bad trouble, this code will fall apart sooner or later.

To check whether I do not have multiple connections or whether the connection/disconnection was successful, I am using these my helper utils https://github.com/vladimir-kraus/qtutils/blob/main/qtutils/safeconnect.h

PS: In the text above I am using term "emitter" (emits the signal) and "observer" (observes the emitter and receives the signal). Sometimes people use "sender" and "receiver" instead. My intention was to emphasize the fact that the emitter emits a signals without actually knowing whether anyone receives it. The word "sender" gives the impression that you send the signal to someone, which is however exactly the cause of signal-slot overuse and bad design. So using "sender" only leads to confusion, IMO. And by using "observer" I wanted to emphasize that signals and slots are the tool to implement the Observer design pattern.

PPS: Signals and slots are also the perfect tool for async communicating between threads in Qt. This use case may be one of the very few exceptions to the principles which I described above.

CodePudding user response：

Signals and slots mechanism is a central feature of Qt.
In general, signals and slots are preferred/used because:

• They allow asynchronous execution via queued connections.
• They are loosely coupled.
• They allow to connect n signals to one slot, one signal to n slots and signal to another signal.

In your project, if signal-slot mechanism has been used to achieve the above, then it is likely the right usage.
GUI input handling isn't the only place where signal-slot mechanism is used.
Unless we know your project's use cases it is difficult to comment if the signal-slot mechanism has been misused/overused.