CODEWITHSUNDEEP
c++memory-managementvector
user997112
user997112

Reputation: 30615

Object being copied in to a vector being cleared because original pre-copy is clear()ed

I have something similar to:

#include<vector>

using namespace std;

vector<char> temp;
vector<char> allbytes = GetBytes();
vector<MyClass> outsidecontainer;

for(int i=0; i<allbytes.size(); i++){

    //Populate my buffer
    if(something){
        temp.push_back(allbytes[i]);
    }

    //temporary buffer now needs to be used to create MyClass object 
    //and outside container store this MyClass object
    else{
        MyClass m(temp);
        outsidecontainer.push_back(m);

        //Empty the temporary buffer ready for next population
        temp.clear();
    }
}

class MyClass{
    public:
    MyClass(vector<char> Message);

    private:
    vector<char> Message;
};

The problem is that come the end, outsidecontainer holds empty MyClass objects. In other words, temp has been emptied due to clear(). However, I didnt think this would affect the value in the outsidecontainer because temp is copied in to MyClass m, which is also copied in to outsidecontainer. They are not storing reference or pointers to objects??

How can I implement the above design, being able to use temp to create MyClass objects and clear it for the next population?

EDIT:

Even though MyClass m has loop-scope, does the object added to outsidercontainer remain after the loop has finished because the value was copied in to the array?

EDIT2:

#include "FIXMessage.h"


FIXMessage::FIXMessage(vector<char> message){
    Message = message;
}

FIXMessage::FIXMessage(const FIXMessage& rhs){

}

Upvotes: 0

Views: 180

Answers (2)

AnT stands with Russia
AnT stands with Russia

Reputation: 320531

Your second edit shows this

FIXMessage::FIXMessage(const FIXMessage& rhs){
}

By doing this you explicitly forced FIXMessage copy constructor to copy nothing. This is why all objects you copy with this copy constructor end up empty. And yes, copy constructor is used when adding elements to a vector.

If you want to write your own copy constructor, it becomes your responsibility to carefully copy all subobjects of the class: base subobjects and member subobjects. You have to do it manually. Instead, you completely suppressed all copying. Why?

No wonder the copied objects end up empty.

But the real question here is whether you actually need a manually implemented copy constructor. Maybe the compiler provided one will work fine? There's no way to say without seeing what your FIXMessage class contains.

For example, if your FIXMessage contains a std::vector and nothing else, the you don't have to write the copy constructor at all. The compiler will provide one for you, which will copy everything correctly.

Finally, learn to use references and initializer lists

FIXMessage::FIXMessage(const vector<char> &message) : Message(message) 
  {}

Passing heavy objects, like std::vector, by value doesn't make much sense, unless you have a very good reason to do so.

P.S. So the original code you posted was fake. Don't post fake code. It can only waste people's time.

Upvotes: 2

Oli_G
Oli_G

Reputation: 510

I might be wrong as we dont know what's going on in your MyClass constructor, but your constructor parameter and vector member share the same name... This is probably the problem.

I just gave a try to your code with this MyClass :

class MyClass{
    public:
    MyClass(vector<char> Message)
    {
        m_Message = Message;
    }

    private:
    vector<char> m_Message;
};

and the outsidecontainer vector still contains all values entered at the end of the loop i.e after many temp.clear();

Hope this helps

Upvotes: 0

Related Questions