Minimum required to emit a signal

[davethomaspilot]

I've been using signals and slots with classes derived from classes created in Qt Designer. In addition to inheriting from the class created with Designer, I inherit from QDialog or QWidget (monkey see monkey do).

I dutifully add the Q_OBJECT in the derived class header file, and everything seems work. I can connect signals of built-in widgets to my own custom slots and emit (and connect) my own custom signals. I must confess I don't understand what's going on "under the covers".

I have some utility classes that really aren't GUI oriented. But, I can see it would be handy for some of the methods in these clases to emit signals that could be connected to new widgets I'd create for displaying some diagnostic information.

I think I need the Q_OBJECT in these class declarations to have a signals: keyword

But, what what do should I inherit from? When I tried to public inherit only from QDialog or QWidget, the compiler complains about not being able to access private data in the base class. And that doesn't seem clean, given my derived class is only similar to a QDialog or QWidget in that it would emit a signal.

So, what do I need just to create a signal?

Thanks,

Dave Thomas

[wysota]

But, what what do should I inherit from?
Inherit QObject.

[davethomaspilot]

Thanks for the quick reply.

I get similar error messages when inheriting from QObject:

1>c:\users\dave\my software\stats helper\heat_message.h(127): error C2248: 'QObject::operator =' : cannot access private member declared in class 'QObject'
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(333) : see declaration of 'QObject::operator ='
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(112) : see declaration of 'QObject'
1> This diagnostic occurred in the compiler generated function 'HeatMessage &HeatMessage::operator =(const HeatMessage &)'
1> main_window.cpp

1>c:\users\dave\my software\stats helper\heat_message.h(127): error C2248: 'QObject::QObject' : cannot access private member declared in class 'QObject'
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(333) : see declaration of 'QObject::QObject'
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(112) : see declaration of 'QObject'
1> This diagnostic occurred in the compiler generated function 'HeatMessage::HeatMessage(const HeatMessage &)'
1> moc_collect_stats.cpp
1>c:\users\dave\my software\stats helper\heat_message.h(127): error C2248: 'QObject::QObject' : cannot access private member declared in class 'QObject'
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(333) : see declaration of 'QObject::QObject'
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(112) : see declaration of 'QObject'
1> This diagnostic occurred in the compiler generated function 'HeatMessage::HeatMessage(const HeatMessage &)'
1> moc_main_window.cpp
1>c:\users\dave\my software\stats helper\heat_message.h(127): error C2248: 'QObject::QObject' : cannot access private member declared in class 'QObject'
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(333) : see declaration of 'QObject::QObject'
1> c:\opencv\dep\qt\qt-everywhere-opensource-src-4.8.2\include\qtcore\../../src/corelib/kernel/qobject.h(112) : see declaration of 'QObject'
1> This diagnostic occurred in the compiler generated function 'HeatMessage::HeatMessage(const HeatMessage &)'
1> Generating Code...
1>
1>Build FAILED.
1>
1>Time Elapsed 00:00:07.10

It compiles fine without the Q_OBJECT and inheritance from QObject.

Do I need a different constructor that passes 0 to the Bo

Here's the relevant part of header file:



#ifndef HEAT_MESSAGE_H
#define HEAT_MESSAGE_H

#include <QtCore/QCoreApplication>
#include <QRegExp>
#include <QMap>
#include <iostream>
#include <fstream>
#include <QTextStream>
#include <QWidget>

class HeatMessage : public QObject
{
Q_OBJECT

public:
HeatMessage(QByteArray &,QObject *parent=0);
HeatMessage(void);
QList<HeatRecord> heat_records;
float get_time(char lane, char id);
float get_sum_time(char lane);
char get_result(char lane);
int get_race(void) {return heat_records.begin()->get_race();};
int get_heat(void) {return heat_records.begin()->get_heat();};
int num_dogs(char lane);
static void print_header(QTextStream &);
void print(char lane, QTextStream &);
void print(QTextStream &);





};
#endif

[ChrisW67]

The QObject copy and assignment constructors are declared private. You cannot copy a QObject and will get these sort of errors if you attempt to do that.

[wysota]

You can't create copies or assignments to QObject instances. You probably have an assignment operator in your class (or the compiler tries to create one for you based on your code). This is not possible.

[davethomaspilot]

Hmm, I don't see anthing like that, unless the following fragment (which is called from a method in a different QWidget class)




HeatMessage hm(msg);
emit new_heat(hm);



Do you have a suggestion on how to find the offending code?

Thanks,

Dave Thomas

[wysota]

What is in heat_message.h file in line 127 (and around)?

[davethomaspilot]

That's the last line in the header file (the };)

Not very useful.

[code]

void print(char lane, QTextStream &);
void print(QTextStream &);



line 127: };

[wysota]

Please post the whole file as an attachment.

[davethomaspilot]

Here it is:

Added after 27 minutes:

A HeatMessage object is passed as the argument emitted signals in a couple of places in other files. If I eliminate those calls, the complaints about referencing a private assignment operator in the base class is eliminated. However, I get some unresolved externals at link time:

1>heat_message.obj : error LNK2001: unresolved external symbol "public: virtual struct QMetaObject const * __thiscall HeatMessage::metaObject(void)const " (?metaObject@HeatMessage@@UBEPBUQMetaObject@@XZ)
1>heat_message.obj : error LNK2001: unresolved external symbol "public: virtual void * __thiscall HeatMessage::qt_metacast(char const *)" (?qt_metacast@HeatMessage@@UAEPAXPBD@Z)
1>heat_message.obj : error LNK2001: unresolved external symbol "public: virtual int __thiscall HeatMessage::qt_metacall(enum QMetaObject::Call,int,void * *)" (?qt_metacall@HeatMessage@@UAEHW4Call@QMetaObject@ @HPAPAX@Z)
1>debug\\Stats Helper.exe : fatal error LNK1120: 3 unresolved externals
1>

Maybe those are result of my quick hack to eliminate code that perhaps caused an implicit copy of a HeatMessage object.

But, now I'm thinking this isn't such a good idea, if I can't pass QObjects as arguments of emitted signals. Is that, in fact, a limitation I need to deal with. Would passing by reference avoid the issue?

Thanks,

[wysota]

How are you using both these classes? Can you show us some code that creates instances of them, etc.?

One thing that I can immediately see that it is incorrect is this:


QList<HeatRecord> heat_records;

This requires HeatRecord to be copyable, which it is not. You can only have this:


QList<HeatRecord*> heat_records;

[davethomaspilot]

Changing the signal prototypes to use HeatMessage & instead of HeatMessage (and one of my methods with a HeatMessage arg) also eliminated the private data compile errors. However, i still get the unresolved externals. Chasing that now.

heat_message.obj : error LNK2001: unresolved external symbol "public: virtual struct QMetaObject const * __thiscall HeatMessage::metaObject(void)const " (?metaObject@HeatMessage@@UBEPBUQMetaObject@@XZ)
1>heat_message.obj : error LNK2001: unresolved external symbol "public: virtual void * __thiscall HeatMessage::qt_metacast(char const *)" (?qt_metacast@HeatMessage@@UAEPAXPBD@Z)
1>heat_message.obj : error LNK2001: unresolved external symbol "public: virtual int __thiscall HeatMessage::qt_metacall(enum QMetaObject::Call,int,void * *)" (?qt_metacall@HeatMessage@@UAEHW4Call@QMetaObject@ @HPAPAX@Z)
1>debug\\Stats Helper.exe : fatal error LNK1120: 3 unresolved externals

[wysota]

Always pass pointers to QObjects in signals, never references nor copies.

The errors you posted are related to not running qmake after adding Q_OBJECT macro to a class.

[davethomaspilot]

Example usage:

connect(this,SIGNAL(new_heat(HeatMessage &)),collect_stats,SLOT(new_heat(HeatMessage &)));

Signal emitted after constructing the HeatMessage like this:


if (msg[len - 7] == 'X')
i
{
HeatMessage hm(msg);
emit new_heat(hm);
fd = fopen("console_data.txt","a");

fprintf(fd,"%s\n", msg.data());
fclose(fd);

msg.clear();
}


That's after changing the signal to use a HeatMessage & instead of a HeatMessage. Compiles now, doesn't link. And, I think it would segfault anyway, since hm will go out of scope before it's used in the slot.


Always pass pointers to QObjects in signals, never references nor copies.

Our posts crossed. I'll try that, and

DOH! of course. I'll redo the qmake.

Thanks,

Dave Thomas

So, I guess I'd have to do a "new" to create a HeatMessage, pass the resulting pointer in the signal, then delete in the slot?

Or, will some other memory management mechanism try to handle it since it's a Q_OBJECT?

[wysota]

So, I guess I'd have to do a "new" to create a HeatMessage, pass the resulting pointer in the signal, then delete in the slot?
It depends on the object. HeatMessage doesn't need to inherit QObject as far as your code is concerned. It doesn't have any signals or slots. Same goes for HeatRecord. Maybe we misunderstood each other. I understood that you wanted some objects of yours to be able to emit signals but now I see that you wanted them to be emitted as arguments to signals. Is that correct?

[davethomaspilot]

No, I want methods in the HeatRecord and/or HeatMessage to emit signals in addition to objects of these types to be passed as arguments.

Those classes parse text data and understand a specific text data format. I'm adding some new syntax to include additional diagnostics about the RF channel integrity and locating the new code that recognizes the new packet signatures seem to make the most sense.

Rather than just printing to QDebug(), I was going to add a widget with slots to display that data.

So, yes I DO want to send signals from methods in a HeatMessage and/or HeatRecord class. But, maybe it's not worth it, if doing that implies more complicated memory management, since I now know signals can contain only pointers to QObjects, not references or copies.

Thanks,

Dave Thomas

Sorry for the confusion--I just deleted the signals: from the header file until I got a successful build with the Q_Object.

The method I'd add would simply send a HeatRecord to a slot in a different class if the content matched a specified RegExp.

[wysota]

No, I want methods in the HeatRecord and/or HeatMessage to emit signals in addition to objects of these types to be passed as arguments.
So what sense does it make to create an object just to emit it in a signal and destroy it right away? What signals could it possibly emit? I think you have a serious design problem... Maybe instead of inheritance you should use composition? I mean have some persistent object that emits signals "on behalf" of value-based HeatRecord and HeatMessage objects.

[davethomaspilot]

I don't create an object just to emit a signal. The object consists of data and methods to parse the data to get interesting data.

The entire object is passed to slots. Not just the data, but the methods to access the data in convenient ways are encapsulated with the data--standard object oriented design. The objects don't exist for long, but so what? You're suggesting that they shouldn't be objects because of this?

But maybe that's a consideration for QObjects? And for that reason, don't create them in situations like this?

I think you're saying it doesn't make sense to inherit from QObject just to send a signal. Fair enough.

Thanks,

Dave Thomas

[wysota]

I don't create an object just to emit a signal. The object consists of data and methods to parse the data to get interesting data.

You said yourself:

So, I guess I'd have to do a "new" to create a HeatMessage, pass the resulting pointer in the signal, then delete in the slot?

Which means you want to create an object, emit it and destroy it. So the only two objects that could possibly connect to a signal emitted by this short-lived object are the sender and the receiver. And both of them can query the state of the object directly, without signals.


The objects don't exist for long, but so what? You're suggesting that they shouldn't be objects because of this?
I'm suggesting they shouldn't be emitting any signals.


I think you're saying it doesn't make sense to inherit from QObject just to send a signal. Fair enough.
No, that's not what I'm saying. I'm saying it doesn't make sense to emit a signal that nobody can use.

[davethomaspilot]

The constructor for the HeatMessage looks at the passed QByteArray to see if it looks like a set of valid HeatRecords This includes making sure it has an integral number of 16 bytes, and finding record boundaries for creating HeatRecords.

HeatRecords are constructed from data passed to the HeatMessage constructor and added to a list of all the HeatRecords found in the HeatMessage.

The HeatRecord constructor parses the bytes passed and saves data in private data members. Methods of the HeatRecord class allow encapsulate access of things like race number, heat number, win/loss--insulating code that wants that information from having to know the byte layout of a HeatRecord or HeatMessage. Sure, code that creates the HeatMessage could dive right into the data, but knowledge of the data format should be encapsulted in the HeatMessage class. That's the motivation for creating an object, not for simply creating a signal.

However, I'm now adding a new record to a communication protocol, that's sort of like "out of band" data. It contains things like "number of retries", RF signal strength, "number of dropped pakets" It isn't a valid HeatRecord (which is a data format I can't modify), and I'd like to show its data in another Widget (to be developed). I'm considering using a signal, so a connect can be used to send these new type records to a widget that extracts data from and displays these new type of records.

So, the motivation for creating a signal is to allow the data parsed in the HeatMessage/HeatRecord constructor to be consumed by other widgets as defined in the widgets that create the HeatMessages.

So, yes, the object would get created, used in the slot code, then discarded. But much more is going on than simply sending the signal. Performance is not an issue-- the HeatMessages are only a few hundred bytes and get created no more frequently than a few minutes apart.

Why shouldn't I be emitting any signals? Isn't this what they are designed fo? The signal indicates "bytes matching this signature were found", and provides the object which has this data along with the surrounding records and accessor methods for display by unspecified widget. Association with a widget(s) is done not by the HeatMessage/HeatRecord class, but in classes that create them.

Thanks,

Added after 19 minutes:

"No, that's not what I'm saying. I'm saying it doesn't make sense to emit a signal that nobody can use."

Thinking about it some more--I guess the problem is the connect, since it would need to be done before the object got created for signals emitted during construction to be handled. But, I'd need the pointer to the newly created object to do the connect.

Obviously, I can get it done without using a signal. Just thought that was a "slick" way of doing it. Not so much now.

Thanks,

Dave Thomas

[amleto]

Maybe I misunderstand something, but it seems like your HeatRecord/HeatMessage is not a good construct. As I understand it, you pass data to these things and then the instances may or may not be 'correctly formed' depending on how the bytearray parsing goes. If this is correct then it is not good practice. Can you clarify this?

[davethomaspilot]

"then the instances may or may not be 'correctly formed'"

That is not correct. They will always be correctly formed. Occaisonally, there will be additional records (e.g. when a lower level prototocol had to retransmit a packet because it didn't receive the expected ack) that will exist in the input data that aren't logically part of a HeatRecord or HeatMessage. They are of interest and will be displayed in a different widget that shows diagnostic data about the communications channel. So, the HeatMessage/HeatRecord classes know about the data format, they only know enough about these types of records (they match a specified RegExp) to understand they should be interpreted by code encapsulated in a different class.



But, I can easily execute a public method in the HeatMessage classe that would be something like "bool get_matching_records (RegExp regexp)". If it returned true, the calling method would emit a different signal and pass the HeatMessage object like it does today. Since HeatMessage isn't an Q_OBJECT, there's no issue with passing a copy.

[wysota]

The constructor for the HeatMessage looks at the passed QByteArray to see if it looks like a set of valid HeatRecords This includes making sure it has an integral number of 16 bytes, and finding record boundaries for creating HeatRecords.

HeatRecords are constructed from data passed to the HeatMessage constructor and added to a list of all the HeatRecords found in the HeatMessage.

So HeatRecord instances are purely value based, as their name suggests -- a record is some set of data. I don't see what sense it makes for data to emit any signals. Even assuming that data in each of those objects might be changed dynamically, who would be interested in connecting to all those objects to monitor those changes?


The HeatRecord constructor parses the bytes passed and saves data in private data members. Methods of the HeatRecord class allow encapsulate access of things like race number, heat number, win/loss--insulating code that wants that information from having to know the byte layout of a HeatRecord or HeatMessage. Sure, code that creates the HeatMessage could dive right into the data, but knowledge of the data format should be encapsulted in the HeatMessage class. That's the motivation for creating an object, not for simply creating a signal.
Still, no use for signals. An object is constructed and there it is.


However, I'm now adding a new record to a communication protocol, that's sort of like "out of band" data. It contains things like "number of retries", RF signal strength, "number of dropped pakets" It isn't a valid HeatRecord (which is a data format I can't modify), and I'd like to show its data in another Widget (to be developed). I'm considering using a signal, so a connect can be used to send these new type records to a widget that extracts data from and displays these new type of records.
Where would that signal be? What would it carry? Who would connect to it? If this signal is to be emitted by HeatMessage then why don't you just add a list of those "out of band" records to HeatMessage class just like you have a list of HeatRecord instances? Each message either has those OOB records or not, it's not like such record would magically appear 10 seconds after the message that contains it has already been parsed.


Why shouldn't I be emitting any signals?
I'm not saying you shouldn't be emitting any signals. I'm saying you shouldn't be emitting signals from HeatMessage and HeatRecord objects.

The way I see it you should have something similar to QNetworkAccessManager -- there is a single "manager" object that spits out HeatRecord objects (and possibly HeatMessage objects however I don't really see a use for them -- who cares whether a particular record was contained in message A or A+1?). When it does that it can emit a "hey, I just created a new HeatRecord object" or a "hey, I just created a new OutOfBandRecord object".

[davethomaspilot]

Wow, lots of stuff in this reply. Thanks!.

I'm not trying to be argumentative. I'm definitely a newbie and want to learn important design considerations. So, thank you for patience and continued replies.

I'll try to clarify what I had in mind with answers to your comments.


So HeatRecord instances are purely value based, as their name suggests -- a record is some set of data.

I don't know what you mean by is "purely value based". There are methods in that class too. What else is missing (data + methods) that would make it appropriate to emit a signal?


"Still, no use for signals. An object is constructed and there it is."

Ok, sounds like the conclusion should be a signal that's generated just once is not worth doing? Depends on what you're trying to accomplish with the signal.

The idea was to make records available for consumption by unspecified (unspecified by the HeatMessage class) widgets. Creators of HeatMessage/HeatRecords would decide whether that signal is of interest and to what (if any Widgets). But, alternately, I can add a method that can be used to in effect "poll" after each creation of the entire message instead of doing an event driven approach like emiting a signal when/if a record of interest is accumulated. Either way will work.


Where would that signal be? What would it carry? Who would connect to it? If this signal is to be emitted by HeatMessage

The signal would be in the HeatMessage (or HeatRecord) class. It would carry a HeatRecord object.



emit special_record(HeatMessage hm) ;


Versus something like:





HeatMessage hm(msg);

emit new_message(hm);
// new code follows:
if (hm.oob_received())
{
emit special_records(hm);
}

This code fragment that instantiates the HeatMessage object is already a QWidget, so everything works fine.

But, the same arguments you're providing would apply here too, eg:


it's not like such record would magically appear 10 seconds after the message that contains it has already been parsed.

But, if I don't use signals, I'd have to create a method in the class for the QWidget that will display the data, and call it instead. Which means I need to include the header files for classes that would consume the data, to be able to call them here. That's why I wanted to use a signal instead. Why not?





then why don't you just add a list of those "out of band" records to HeatMessage class just like you have a list of HeatRecord instances?

That was what I planned to do anyway. The only difference was that I was just going to generate a signal in the code that will already know it's one of these new record types rather than having to make a call to a method to see if any exist. They will rarely be present.


Each message either has those OOB records or not, it's not like such record would magically appear 10 seconds after the message that contains it has already been parsed.

True.

So, here's what I'm concluding from this thread. Please correct me if I'm wrong.

1) You cannot pass objects derived from Qbjects (or QObject &) as an argument of a signal.
(This is the one that makes it more hassle than it's worth in this situation). I'd like to confirm that understanding.

2) You're recommending NOT creating signals, except if the signals would occur more times than just at object creation time?

The alternative implementation works fine and requires minimal code change too. A separate method is used to return the list of OOB messages and the code that creates HeatMessage instances can call that method and emit the signal if OOB records exist. I don't see why I would NOT emit a signal on what I think of as a rare event, but I could include the header file for the widget that will consume the data instead of emitting the signal. Again, the motivation for using the signal is to allow the binding to between producer and consumer of that data elsewhere, but it's not a strict requirement.

Thanks again.

Dave Thomas

[amleto]

you can pass QObjects through signal/slots but you should do so by using pointer-to-QObject.


That is not correct. They will always be correctly formed. Occaisonally, there will be additional records (e.g. when a lower level prototocol had to retransmit a packet because it didn't receive the expected ack) that will exist in the input data that aren't logically part of a HeatRecord or HeatMessage. They are of interest and will be displayed in a different widget
And now I am more convinced that this data should be encapsulated in a different entity and not grouped with Heat[Record|Message]. You have just said that this data is separate to your record/message AND that it has a different purpose. It seems obvious to me that a new struct/class is desired.



Creators of HeatMessage/HeatRecords would decide whether that signal is of interest and to what (if any Widgets).

This dependency is backwards in one sense, and simply not necessary in another. The creator(s) should be agnostic of consumers (the tail should not wag the dog). The creators should make and emit the records. The clients should receive and do what they want with them. A dependency would be injected between the creator and the consumers to form the signal/slot connections - ensuring the creators/consumers are mutually agnostic.

[wysota]

I don't know what you mean by is "purely value based".
I mean it has no identity. If you have two or more objects you can compare their data and say "yes, there are identical" or "no, they are different, because...". With identity-based objects no two objects are identical.


There are methods in that class too.
It doesn't matter. QColor also has methods but it is value-based (its role is to carry some value).


Ok, sounds like the conclusion should be a signal that's generated just once is not worth doing?
I really don't see any signal here. There is no "hey, I've just been created" signal since nobody has yet a chance to connect to it before it is emitted. If you access an object, it's no surprise it has been created.


The idea was to make records available for consumption by unspecified (unspecified by the HeatMessage class) widgets.
Emitting a signal does not make anything "available for consumption". If you have 10 slots connected to this signal, which one will "consume" the argument of the signal? If you want to make objects available for consumption then you provide a getter to fetch that object.


Creators of HeatMessage/HeatRecords would decide whether that signal is of interest and to what (if any Widgets).
It is contradictory with the nature of signals. A signal makes an object say "hey, something has happened to me" without checking if anyone cares about it or not.


The signal would be in the HeatMessage (or HeatRecord) class. It would carry a HeatRecord object.
So one HeatRecord would emit another HeatRecord? How would that work? Or would the HeatMessage do that? When would it do it? Take a piece of paper, draw a line representing the lifetime of your HeatMessage object, mark the place where the object is born, mark the place where it is emitted from whatever emits it and try to mark a point in time where it would emit information that it contains an OOB record. Then think what is meant to happen (in terms of code) between the time the message is born and the time when it emits this OOB record.




emit special_record(HeatMessage hm) ;


Versus something like:





HeatMessage hm(msg);

emit new_message(hm);
// new code follows:
if (hm.oob_received())
{
emit special_records(hm);
}

I don't see HeatMessage emitting any signals here.


But, if I don't use signals, I'd have to create a method in the class for the QWidget that will display the data, and call it instead. Which means I need to include the header files for classes that would consume the data, to be able to call them here. That's why I wanted to use a signal instead. Why not?
You still don't get it. I'm not saying you shouldn't inform the environment about existance of a OOB record by emitting a signal, I'm saying you shouldn't be emitting it from HeatMessage because at the time you emit it, nobody has yet a chance to connect to that signal (which is a conclusion you should be reaching as a result of the piece of paper exercise). The OOB record IS or IS NOT, there is no situation that you parse a message and there is no OOB record but 1 CPU tick later it magically appears as part of the message object. It's either there from the very beginning of existance of that message object or it will never be in that object.


1) You cannot pass objects derived from Qbjects (or QObject &) as an argument of a signal.
False. You can't pass them by value and you shouldn't pass them by reference. You can pass by pointer but you have to make sure no slot connected to such signal tries to delete this object.


2) You're recommending NOT creating signals, except if the signals would occur more times than just at object creation time?
False. QThread::finished() is a one-time signal yet it's a perfectly valid one. I'm recommending not to emit signals from objects which are value-based (as opposed to identity-based).

[davethomaspilot]

you can pass QObjects through signal/slots but you should do so by using pointer-to-QObject.

Yes, that's what I was trying to say. Which puts limitations on using them as arguments of signals, due to the private assignment operater in the Qobject as detailed earlier in the post


And now I am more convinced that this data should be encapsulated in a different entity and not grouped with Heat[Record|Message]. You have just said that this data is separate to your record/message AND that it has a different purpose. It seems obvious to me that a new struct/class is desired..

Well, they are encapsulated in a separate list contained in the same HeatMessage classe. It just made sense to identify them as these "special" or "not mine" records in the same code that is categorizing each of the various HeatRecord types. (I still do). Sure, I could make that list in a different class, but that really isn't relevant to the signal/no-signal topic we're focusing on here. The records are available in the separate list, the signal was just a means to create an event that could be connected by anything interested in the fact these records were available.


This dependency is backwards in one sense, and simply not necessary in another. The creator(s) should be agnostic of consumers (the tail should not wag the dog). The creators should make and emit the records. The clients should receive and do what they want with them. A dependency would be injected between the creator and the consumers to form the signal/slot connections - ensuring the creators/consumers are mutually agnostic.

Hmm, nearly every Qt example I've seen does something like instance objects then connect signals from those objects. That's what I was doing. Versus making a direct call in the HeatMessage class. So, using a signal just sets "special records are in this HeatMessage, for anyone interested. Connection to be made by the code instancing the object (or anywhere else). The point is NOT to use a direct call in the HeatMessage class.

Added after 10 minutes:


Emitting a signal does not make anything "available for consumption". If you have 10 slots connected to this signal, which one will "consume" the argument of the signal? If you want to make objects available for consumption then you provide a getter to fetch that object.

All 10. Getter methods are part of the object passed for any/all connected slots.

1) You cannot pass objects derived from Qbjects (or QObject &) as an argument of a signal.
False. You can't pass them by value and you shouldn't pass them by reference. You can pass by pointer but you have to make sure no slot connected to such signal tries to delete this object.[/QUOTE]

Ok, understood. Not by reference since the referenced data may go out of scope before the slot code has a chance to access it, right?


You still don't get it. I'm not saying you shouldn't inform the environment about existance of a OOB record by emitting a signal, I'm saying you shouldn't be emitting it from HeatMessage because at the time you emit it, nobody has yet a chance to connect to that signal (which is a conclusion you should be reaching as a result of the piece of paper exercise). The OOB record IS or IS NOT, there is no situation that you parse a message and there is no OOB record but 1 CPU tick later it magically appears as part of the message object. It's either there from the very beginning of existance of that message object or it will never be in that object.


I'm saying you shouldn't be emitting it from HeatMessage because at the time you emit it, nobody has yet a chance to connect to that signa.

Yes, I understood/understand that. Stating it differently, the issue is that the connect needs a pointer which isn't available until after the object is created.

But what I'm doing now works and still creates a signal passing a value based object after that alue based object gets created. And, conditionally another signal is generated if the object has records matching a specified RegExp. Why signals? To be agnostic of what code (if any) is interested in that data.

Why not?

[wysota]

All 10. Getter methods are part of the object passed for any/all connected slots.
I do not understand this statement.



Ok, understood. Not by reference since the referenced data may go out of scope before the slot code has a chance to access it, right?
That's one of the cases, yes. Another is simply that you can't store a copy of a reference (which is what is required with cross-thread signal-slot connections) --- you'd get and modify a copy of the object (and we already know QObject can't be copied anyway).


Yes, I understood/understand that. Stating it differently, the issue is that the connect needs a pointer which isn't available until after the object is created.
I'm focusing more on logical issues, not technical ones. Even if you could get this pointer somehow (I believe the whole construction is possible to achieve), it simply wouldn't make any sense because the signal would have to be emitted right after the object was constructed because that's when it gets its info about the embedded OOB record. Since we already have a pointer to that object (because we just connected to it), we can simply query the object directly.


But what I'm doing now works
My favourite answer to that is that this works too:


const char *txt = QByteArray("abc").constData();
printf("%s\n", txt);

but it is not correct.


Why signals? To be agnostic of what code (if any) is interested in that data.
You are not becoming data agnostic just by putting signals here and there. With signals being agnostic means that one object doesn't care what other objects might do with a signal it emits and it has to work both ways -- the receiver can't care where its input data comes from. In your case you're emitting a signal for a specific purpose, so it is not agnostic.

[davethomaspilot]

All 10. Getter methods are part of the object passed for any/all connected slots.
I do not understand this statement.


The HeatMessage class will have a method to get the list that contains records that match the specified Regexp. The HeatMessage object is passed as an argument of the signal. Slot code won't have to know about the byte layout of the HeatMessage, they execute methods of the passed object to get what they need and use to populate their own standard widgets, for example.


You are not becoming data agnostic just by putting signals here and there. With signals being agnostic means that one object doesn't care what other objects might do with a signal it emits and it has to work both ways -- the receiver can't care where its input data comes from. In your case you're emitting a signal for a specific purpose, so it is not agnostic.

Hmm, it seems to me there is value in the signal creator to not what is done (if anything) when that signal is emitted.
being agnostic means that one object doesn't care what other objects might do with a signal it emits... I think emitting a signal does that and it's useful. If for no other reason, I can complete and test the code that instances the object and creates the signal before the data that processes the data sent by the signal is completed. Just because it "doesn't work both ways" doesn't mean it's no value to emit a signal, does it?

And, the receiver (slot code) doesn't care where the data came from either. The same HeatMessage could come (and does) from a text file instead of a class data from a QSerialPort.

[lanz]

Your design is incorrect at a higher level, so you're trying to solve wrong problem.
wysota suggests (as I have understood) more clean design:

class NetworkManager {
...
signals:
void NewHeatMessage (HeatMessage msg);
...
void ParseStream (QByteArray *data) {
HeatMessage hm = form_message (data);
emit NewHeatMessage (hm);
};
};

class DataConsumer {
...
public slots:
void ProcessHeatMessage(HeatMessage msg){
while (msg.has_records ()) {
HeatRecord hr = msg.TakeRecord ();
do_stuff (hr);
};
};
};

class DebugWidget {
...
public slots:
void DisplayOOB (HeatMessage msg) {
if (msg.hasOOB ()) {
do_display (msg.GetOOB ());
};
};
};

[wysota]

I think emitting a signal does that and it's useful

You wanted to create an object, emit it and then delete it in a slot. It is everything but being agnostic.

Nevertheless we still haven't seen what signals you wanted to emit from HeatMessage and when. I suggest you just try to do that and see for yourself if it works for you or not. So far you were just emitting HeatMessage and not emitting from HeatMessage.

[davethomaspilot]

The design indicated by Lantz is close to what I have. The difference being that I issue two different signals, one for ALL heat messages that is connected to the equivalent of Data Consumer. The other signal is generated ONLY when these OOB records were discovered in the message during construction. This second signal may be connected to one or more different consumers.





HeatMessage hm(msg);
emit new_heat(hm);

if(hm.extra_records())
emit extra_records(hm);



I was thinking I create the second, relatively rare signal in the HeatMessage constructor after a HeatRecord was added to the list, IFF the record matched a RegExp that was set up as class static data before construction any HeatMessages. (That's still possible, but clumsy, I think), Instead, what I have now looks at the HeatMessage after construction and sends the second signal only if those new type of records were seen in the HeatMessage.

So, my original design (pre-new record types introduced) is unchanged, except new code is added to HeatMessage to recognize and save records of the new type, and a second signal is conditionally sent by the code that instances the HeatMessage.

Yes, I was thinking of emiting the signal IN the HeatMessage construction, instead of after it was created. I agree it's better to do it after the object has been constructed. Wysota, does that adequately answer "Nevertheless we still haven't seen what signals you wanted to emit from HeatMessage and when?". Not sure how else to explain it.

As far as being "everything but agnostic". Fine. So, what's the right term to use? A signal is available for connection by any interested client. Using a signal to that make sense to me for the reasons I've detailed. Having a second signal seems much better than sending EVERY message to clients who are only interested in record types that will very rarely be present in the message.

Anyway, thanks for all the feedback.

[wysota]

Wysota, does that adequately answer "Nevertheless we still haven't seen what signals you wanted to emit from HeatMessage and when?".
No, it doesn't explain it at all. I don't see any practical (or even theoretical) sense of emitting a signal from within a constructor.

[davethomaspilot]

Ok, you don't see a good reason to do it "from a constructor" and I don't disagree. I was just trying to make sure I didn't leave out something you wished to see:



"Nevertheless we still haven't seen what signals you wanted to emit from HeatMessage and when?".

That seemed a little different than convincing you that it makes practical or theoretical sense of doing so.

I agree, it's not a good idea.

Your comment:


being agnostic means that one object doesn't care what other objects might do with a signal it emits...

Was that specific to when I was considering emitting the signal in the HeatMessage/HeatRecord constructor? Or, do you still think what I'm now doing is still not "agnostic"?



Thanks,

Dave Thomas

[wysota]

I was just trying to make sure I didn't leave out something you wished to see:
I thought this was the subject of this thread -- that you wanted to emit a signal from HeatMessage. If it is just an academic discussion, I don't see why we should waste more time on this.

[davethomaspilot]

Yes, that was the main thrust and what I was considering. I just wanted to make sure I wasn't missing something else important about my design. The comment about bad design, not agnostic, etc were specific to trying to emit a signal in the constructor.

Thanks again.