Jump to content
  • Advertisement
Sign in to follow this  
drwhat

Newbie Question About Std:vector

This topic is 855 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I am trying to change my code from dynamically managing arrays I create myself  to using std::vector<>.   Every time I use push_back() it seems to erase all the elements in the array if it has to expand.

Class A
{
// lots of stuff
}

Class B
{
// lots of stuff
}

Class Problem
{
//constructor for this class is empty;
Problem();
~Problem();
void Initialize(A* aPtr, B* bPtr);
A* m_aPtr;
B* m_bPTr;
}

// Code from the main area.

std::vector<Problem> m_problemArray;

//This code works perfectly and I Visual Studio shows the all the data there.
m_problemArray.push_back(Problem());
m_problemArray[0].Initialze(aPtr,bPtr);

// At this point the vector needs to resize to handle a new element
// When I debug it shows m_problemArray[1] has all the correct data but the first 
// elmenent has been erased.
m_problemArray.push_back(Problem());
m_problemArray[1].Iniitalize(aPtr,bPtr);

//If i  do the following line m_problemArray shows its now size 3 but all
//elements are erased
m_problemArray.push_back(Problem());

I read most of the documentation on std::vector.  And I don't see why using push_back on a full array should cause the original elements to be erased?  

 

EDIT:  When I try the following

m_problemArray.assign(2,ProblemClass());
m_problemArray[0].Initialize(aPtr,bPtr);
m_problemArray[1].Initialize(aPtr,bPtr);

and don't use push_back at all everything works correctly.  So there is something funky with the push_back and re-allocating memory.

Edited by drwhat

Share this post


Link to post
Share on other sites
Advertisement
Because vector is a dynamic array, adding items (calling push_back) can force the vector to reallocate its internal storage. When it does this it needs to copy the elements to the new storage. This will involve, ultimately, destroying the original elements, so that is why you'd see destructors invoked.

You don't appear to implement a copy or move constructor or copy/move assignment operator, and your class contains pointers. This means it can't be copied or moved correctly, and is the likely source of your trouble.

Implement those operations; look up the "rule of three" for more. Also you may want to check out vector's "reserve" function.

Share this post


Link to post
Share on other sites
If you have a reasonably modern compiler (that is, not half a decade old or more) then you do not need full copy semantics (these can be fairly difficult to get right because they mess with strong ownership semantics).

In any case, unless you know definitely that a class should be able to be copyable, you should forbid it:
class MyClass
{
   MyClass(const MyClass&) = delete;
   MyClass& operator = (const MyClass&) = delete;
};
see also boost::noncopyable.

If you forbid copying a class, then you need to make it movable to use it in a vector, see move constructors and move assignment.

Another alternative is to manage m_aPTr and m_bPTr using std::shared_ptr. I strongly advise against getting used to that too much. A key skill in software development is deciding (and enforcing) who is responsible for what and a shared_ptr hides this problem. That can lead to some pretty messy and annoying systems but when starting out it might be a useful bandaid to avoid a lot of problems to hit you all at the same time. As long as you realize it's still a bandaid and not a solution for everything it's probably not too bad.

Also, when you need to initially fill a vector it is a good idea to call reserve first with a reasonable value to avoid excessive reallocations.

Share this post


Link to post
Share on other sites

@Josh: Thank you very much.  I am trying to learn DirectX11 and using C++.  I have only programmed in C (where you didn't have a lot of templating) and  C# where a lot of this was handled behind the scenes.   Implementing the Copy Constructor worked perfectly.   

 

@BitMaster:  Making a simple copy constructor to copy the pointers worked fine for this simple case, but I"m looking into the move constructors and the idea of making things not copyable.   As far pointers i think I'll avoid shared_ptr.  I like knowing in each class if a pointer is one this class created and should be deleting, or if it was passed in and another class will be repsonsible for it.

 

Thanks to you both for the quick and helpful repsonses.

Share this post


Link to post
Share on other sites

If you have a reasonably modern compiler (that is, not half a decade old or more) then you do not need full copy semantics (these can be fairly difficult to get right because they mess with strong ownership semantics).


Can you explain your thinking here? I'm guessing that you're referring to modern compilers using move semantics within vector resize operations instead of copying, but I thought the move constructor would require that the copied object gets its pointers nulled-out (to avoid the subsequent destructor call freeing the relinquished memory), and the default move constructor wouldn't do that, right?

(Do point out if I'm missing something - I'm not too familiar with C++11.)

Share this post


Link to post
Share on other sites
A valid move constructor/assignment operator needs to leave the object moved out of in a "valid but unspecified state". Actually the language does not require that but anything which you want to put into a standard library container needs to do that and I have also never seen a scenario where that was not possible.

One common pattern to do that is to implement the move constructor by delegating to the default constructor and then swapping the members.

I'd say the core problem is the OP described the problem in a too general way. I strongly suspect m_aPtr and m_bPtr are resources owned by Problem. In this case the solution is extremely simple: use std::unique_ptr instead of raw pointers and everything fixed itself. Problem is no longer copyable (because unique_ptr are not) but Problem is movable (because unique_ptr are and the compiler will create a default move-constructor based on the unique_ptr move semantics).
That's usually the best case: you have a complicated class but you do not even need to bother making it copyable and/or movable because the member data already enforces what it needs/allows.

Edit: If you find yourself explicitly writing a lot copy/move constructor or assignment operators that's usually a sign of either of two things:
1) you are doing something wrong.
2) you are writing a lot of very low-level primitives (similar in scope to for example unique_ptr or shared_ptr) in preparation to use them in your more complex classes. Edited by BitMaster

Share this post


Link to post
Share on other sites

That 'initialize' is making my teeth grind.

 

drwhat, do you know about member initialization lists?

Share this post


Link to post
Share on other sites

A valid move constructor/assignment operator needs to leave the object moved out of in a "valid but unspecified state".


But the example we're talking about does not do that, if the pointers are implying ownership. The object that has had resources moved from it still believes it is referencing that data, and will destroy it in the destructor.

I guess what you were saying is that, if you use std::unique_ptr and the like, then you wouldn't need to implement the copy semantics. But when the class in question has raw pointers like the one above, obviously the writer has to consider exactly what sort of semantics they're working with.

Share this post


Link to post
Share on other sites

A valid move constructor/assignment operator needs to leave the object moved out of in a "valid but unspecified state".


But the example we're talking about does not do that, if the pointers are implying ownership. The object that has had resources moved from it still believes it is referencing that data, and will destroy it in the destructor.


Obviously he would have to write move semantics which respect that.

In my first post I was only working with what the OP supplied but by the time I wrote my second post I felt the abstract problem description of the OP needlessly complicated the whole issue.
It's really the case that you normally don't have to write copy/move constructors or assignment operators. Either you are happy with just inferring it through the members or you forbid copying/moving using something like boost::noncopyable or '= delete'-ing.

Share this post


Link to post
Share on other sites
I suspect you're expecting a more up-to-date understanding of the language than most people will have. That's a good thing to aim towards, but we need to be careful when saying "you do not need full copy semantics" because they obviously need to replace those with something else, something they are even less likely to understand. eg. I've never seen constructor deletion in academic or professional code, and boost tends not to be used in game code.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!