1

I have the following structs:

enum messageType {New = 0, Old = 1, No_Message = 2};

typedef struct {
    enum messageType type;
    unsigned int senderID;
    char message[100];
} StoredMessageData;

struct StoredMessage {
    unsigned int recipientID;
    vector<StoredMessageData> messages;

    StoredMessage(const unsigned int& intRecipient = 0, const vector<StoredMessageData>& data = vector<StoredMessageData>())
    : recipientID(intRecipient), messages(data)
    {
        messages.reserve(10);
    }

    bool operator<(const StoredMessage& compareTo) const
    {
        return recipientID < compareTo.recipientID;
    }

    bool operator==(const StoredMessage& compareTo) const
    {
        return recipientID == compareTo.recipientID;
    }
};

In my main method, at some point I receive a message and want to store it like so:

if(msgs.find(rcvdRecipientID) == msgs.end())
{
    StoredMessage storedMsg;
    storedMsg.recipientID = rcvdRecipientID;
    msgs.insert(storedMsg);
}

set<StoredMessage>::iterator it = msgs.find(rcvdRecipientID);
StoredMessage storedMsg = *it;

StoredMessageData data;
data.type = New;
data.senderID = rcvdSenderID;
strcpy(data.message, rcvdMessage);

storedMsg.messages.push_back(data);

If, after push_back(), I call storedMsg.messages.size(), I am given a value of 1. This makes sense to me.

However, later, I wish to know how many messages I stored, so, this code:

StoredMessage storedMsg;
if(msgs.find(rcvdSenderID) != msgs.end())
{
    storedMsg = *(msgs.find(rcvdSenderID));
    cout << "Number of stored messages to send: " << int(storedMsg.messages.size()) << endl << endl;
    ...

Here, storedMsg.messages.size() returns 0 even when the same ID as before is used... I am confused as to why this is happening and suspect it has to do with the vectors being copied as their size varies, but I'm not sure. I am not a C++ expert, so please be gentle. Thank you!

Gossamer Shadow
  • 387
  • 1
  • 5
  • 16

1 Answers1

2

Propbably it happens because here

StoredMessage storedMsg = *it;

You create the copy of object and do all the modifications to the copy.

  • Not "probably"; definitely (+1). `StoredMessage &storedMsg = ...` will fix it. But this code is pretty bad in other ways... I suggest instrumenting the `StoredMessage` constructor to see where and how often it is called. (The answer is "a lot" -- e.g. on every call to `find` -- and as a non-trivial constructor, that is a bad thing.) – Nemo Dec 01 '12 at 20:26
  • I agree. I also would mark this contructor with explicit keyword. – son of the northern darkness Dec 01 '12 at 20:29
  • What would I set StoredMessage &storedMsg equal to? I tried setting it equal to *it and it; however those return a const. If my goal is to modify the message, this seems backwards. Is there another way to find that does not return a constant? Also, I had not considered that about the constructor... I will have to modify the way I'm doing that. Thank you. – Gossamer Shadow Dec 01 '12 at 20:35
  • 1
    @GossamerShadow I would use use it->messages.push_back(...). Also you can use references as Nemo suggested. – son of the northern darkness Dec 01 '12 at 21:38