Managing memory with std::vector<somepointer*>

Question

4.00/5 (1 vote)

See more:

Hi,

I've written the following inventory management system, and I've come across a potential memory leak which I'm unsure how to handle. There are two issues here;

1. The std::vector<Item*> items is storing pointers to items which need to be deleted to free memory when the game ends - as it's a member variable, items itself will be deleted when PlayerInventory is deleted, but as for the pointers it contains, I'm not sure how to manage them.

2. In RemoveItemsOfType, I create a new item called nullItem and set the item in the given position to nullItem. Do I need to call 'delete items[slot];' before I do this to free the memory held by the item currently there, before I replace it with nullItem?

#include "Item.h"

void Item::Init(ItemType type)
{
	this->type = type;
}

ItemType Item::GetType()
{
	return type;
}

#include "PlayerInventory.h"

PlayerInventory::PlayerInventory()
{
	for (int i = 0; i < slots; i++)
	{
		Item* item = new Item();
		item->Init(ItemType::NONE);
		items.push_back(item);
	}
}

void PlayerInventory::SetItem(int slot, Item* item)
{
	items[slot] = item;
}

Item* PlayerInventory::GetItem(int slot)
{
	return items[slot];
}

void PlayerInventory::RemoveItemsOfType(ItemType type)
{
	for (int i = 0; i < slots; ++i)
	{
		if (GetItem(i)->GetType() == type)
		{
			Item* nullItem = new Item();
			nullItem->Init(ItemType::NONE);
			SetItem(i, nullItem);
		}
	}
}

What I have tried:

Thinking very hard about what to do but not knowing the answer so coming here for advice :)

Posted 25-May-18 2:21am

User 13204940

Updated 25-May-18 11:27am

v2

Add a Solution

Comments

Richard MacCutchan 25-May-18 8:40am

Why are you putting a new item in the slot? Why not just change the type of the existing one? But if you must do it your way then you need to delete the existing item before you overwrite the entry.

[no name] 25-May-18 8:49am

Hmm. I'm taking the wrong approach here. I need Item to be a base class which other classes can inherit from.

When PlayerInventory is deconstructed, and the member 'items' is deleted, will I need to delete each of the pointers in 'items' in PlayerInventory::~PlayerInventory()?

Richard MacCutchan 25-May-18 9:04am

Basically, yes. If you create an object with new then you must dispose of it with delete. Exactly where you do that will depend on the structure of your application.

[no name] 25-May-18 10:31am

If only I knew the answer while writing the question, maybe I could have gotten a 5-star. Damn it.

Richard MacCutchan 25-May-18 10:36am

Well things are improving, you got a 4 today.

[no name] 25-May-18 10:37am

yay! :>

3 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

CPallini · Answer 1 · 2018-05-25T11:28:00

Use a smart pointer. Try, for instance

C++

#include <iostream>
#include <vector>
#include <memory>
using namespace std;

class F
{
  int f;
public:
  F(int f):f(f) { cout<< "F(" << f << ") ctor called\n";}
  ~F() { cout << "F(" << f  << ") dtor called\n";}
};

int main()
{
  vector< F * > v;
  v.push_back(new F(1));

  vector < unique_ptr< F > > w;
  w.push_back( make_unique< F >(2) );
}

Output:

F(1) ctor called
F(2) ctor called
F(2) dtor called

Rick York · Answer 2 · 2018-05-25T04:34:00

Solution 2

I have found that yes, you do have to manually delete vectors of pointers. If you use Visual Studio and the DEBUG_NEW macro you will see the memory leaks in the debugger if you don't. At least, I do. I came up with this little template function a while ago to help with this situation :

C++

// delete a vector of objects created with new

template< typename T >
void DeleteVector( std::vector<T> & v )
{
    size_t count = v.size();
    for( size_t n = 0; n < count; ++n )
        delete( v[n] );
    v.clear();          // just to be sure
}

If you use a malloc variant to allocate the pointers then you can make a variation of this function that calls free instead of delete and since free is not a typed operation the function does not need to be a template.

Posted 25-May-18 4:34am

Rick York

Updated 25-May-18 4:36am

v2

Comments

[no name] 25-May-18 11:04am

I'll definitely have a good look into this. Would it be a good idea to have all of my classes include a .h with #define new DEBUG_NEW inside so I can collect all usages of 'new' in the entire project? I feel that would give me good insight into memory leaks anywhere they may be.

I've read that std::vector::clear() destroys and deletes each item in the vector, so I believe a simple call to clear() without the loop would be sufficient

jeron1 25-May-18 11:12am

"I've read that std::vector::clear() destroys and deletes each item in the vector,"
That probably applies to memory the vector itself allocates (the 4 or 8 bytes needed to hold your pointers) not the objects that those pointers are pointing to.

[no name] 25-May-18 11:18am

From cplusplus.com: "Removes all elements from the vector (which are destroyed), leaving the container with a size of 0."

jeron1 25-May-18 11:47am

The elements in your vector are pointers. Each 4 or 8 byte entry is removed/destroyed, not the objects they reference. Perhaps set a breakpoint on the destructor on an 'Item' and see if it gets called when your vector is destroyed.

[no name] 25-May-18 11:49am

Great idea, I didn't think of that. I'm actually using Unreal Engine 4 which uses Visual Studio for development, so I don't believe there's a way to set breakpoints but I can certainly output something to the console within the destructor

jeron1 25-May-18 12:02pm

No breakpoints or the ability to step through code? It's got to be in there somewhere, I can't imagine (nor would I consider) doing any development without it.

[no name] 25-May-18 12:05pm

It's a very unfriendly engine. The learning curve is sure to push away most people before they even begin. I've gotten to the point of anger with its uselessness, lack of C++ examples with the documentation or even a clear explanation, and the fact that they only seem to care about the blueprint element while completely disregarding those who want to have better control and program in C++. Everything is very difficult, you can't even have constructors with arguments.

Ask a generic question on google regarding unreal engine, you'll see almost only blueprint examples.

The UE4 way is the only way, no room for per-developer habits.

jeron1 25-May-18 12:15pm

Doesn't sound like fun.

KarstenK · Answer 3 · 2018-05-25T02:54:00

Solution 1

Rule of thumb is, who creates memory has to delete it. So when you remove an vector you must delete the memory.

But you have the additional problem, that your vector may be copied or assigned somewhere deep in the vector implementation and so you may have 2 vectors with the same memory. Maybe it helps to overwrite these operators in which you copy the memory of the sourse vector.

I think you dont need to allocate the Item with new, but can use a typed vector.

C++

std::vector<Item> myList;

Posted 25-May-18 2:54am

KarstenK

Comments

[no name] 25-May-18 9:04am

So in PlayerInventory, I have a member variable std::vector<Item*> items.

When the PlayerInventory is destructed, items will be automatically deleted too. However, does this mean those Item*s' memory is still allocated and need to be deleted within the destructor?

jeron1 25-May-18 10:11am

" in PlayerInventory, I have a member variable std::vector<Item*> items...."
"When the PlayerInventory is destructed, items will be automatically deleted too."
Someone correct me if I'm wrong, but that doesn't sound right to me. I think if you're going to 'new' objects, you need to 'delete' them, probably in the PlayInventory destructor. I'm in agreement with KarstenK on this in using the typed vector, then memory management is out of your hands.

[no name] 25-May-18 10:29am

I've been told that member variables, the std::vector itself, will be deleted when the class is destructed. I don't know whether that automatically deletes the pointers in the vector or not, which are created with 'new'.

the vector is declared as such;

public:
        std::vector<Item*> items;

KarstenK 26-May-18 4:23am

In C++ werent pointer deleted when the container gets deleted. This ONLY happens with smart pointers.

Tip of a seasoned developer: learn and understand the techniques you want to use ;-)

[no name] 26-May-18 10:20am

I understand best by trying, instead of reading a book and having no idea what anything actually translates to in a real project.

Managing memory with std::vector<somepointer*>

3 solutions

Solution 3

Solution 2

Solution 1

Add your solution here

Preview 0

Existing Members

...or Join us