Dealing with memory, when using std::vector

Question

I'm using vector with my own class type:

std::vector<CItem> m_vItems;

In my class I'm initializing SFML types like texture and sprite:

class CItem
{
    (...)
    sf::Texture m_Texture;
    sf::Sprite m_Sprite;
    sf::IntRect* m_pRect;
    (...)
}

I'm trying to pass object to my vector declared as member of other class CLevel and I'm doing it inside method of that class like this:

CItem *temp = new CItem(x, y, kind);
m_vItems.push_back(*temp);

As you see, I'm not deleteing temp pointer with delete, but in destructor of class CLevel I've got a simple line:

std::vector<CItem>().swap(m_vItems);

And I'm little confused about memory leaks. Is my program has got some of these or the line above solving problem and my example has been correctly written?

please insert a code of destructor

4pie0
– 4pie0

2014-02-24 15:57:38 +00:00
Commented Feb 24, 2014 at 15:57 — 4pie0
– 4pie0, Commented Feb 24, 2014 at 15:57
Which destructor? CItem or CLevel?

Gucu112
– Gucu112

2014-02-24 21:20:21 +00:00
Commented Feb 24, 2014 at 21:20 — Gucu112
– Gucu112, Commented Feb 24, 2014 at 21:20

Mark B · Accepted Answer · 2014-02-24 15:58:48Z

2

Your program calls new without a matching delete, and it didn't pass the result of new to some other class that will manage it for you. Therefore your program has a memory leak.

Do you have a problem with using m_vItems.push_back(CItem(x, y, kind)); instead of the two line example you gave?

answered Feb 24, 2014 at 15:58

Mark B

96.6k10 gold badges113 silver badges198 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Gucu112 Over a year ago

When I'm using it like this, after method's end sprite's texture has got incorrect, random values of width and height.

4pie0 Over a year ago

implement this constructor correctly to avoid uninitialized variables

4pie0 · Accepted Answer · 2014-02-24 16:05:14Z

1

CItem *temp = new CItem(x, y, kind);
m_vItems.push_back(*temp); // here a copy of *temp is pushed into vector

You should call delete somewhere to delete what you allocated with temp:

delete temp;

to avoid memory leak. Any call to new must have matching call to delete somewhere. This doesn't influence a copy of temp that was pushed into vector. It still exist as long as vector exists.

The best is to use just:

m_vItems.push_back(CItem(x, y, kind)); // implement this constructor correctly
                                       // to avoid uninitialized variables

Always when leaks are concern you can profile your program with tool: Valgrind or Visual Leak Detector.

edited Feb 24, 2014 at 16:05

answered Feb 24, 2014 at 15:59

4pie0

29.9k10 gold badges86 silver badges126 bronze badges

Comments

mbadawi23 · Accepted Answer · 2014-02-24 15:59:38Z

0

if your are on Windows platform, you could use the CRT library to detect if you have any leaks in a specific code section. This link explains how to do it in VS 2012 it is also available for earlier versions.

answered Feb 24, 2014 at 15:59

mbadawi23

1,0812 gold badges22 silver badges45 bronze badges

Comments

gbjbaanb · Accepted Answer · 2014-02-24 16:05:54Z

0

Assuming you need to keep a vector of pointers, rather than a vector of CItems, I'd use a smart pointer to manage the object lifetime. A shared_ptr is easy to use:

shared_ptr<CItem*> temp(new CItem(x,y,z));
m_vItems.push_back(temp);

When the vector goes, the CItem will be cleaned up correctly. When the items are passed around, they will be handled correctly too - no memory leaks.

answered Feb 24, 2014 at 16:05

gbjbaanb

52.9k12 gold badges111 silver badges157 bronze badges

Comments

user2460683 · Accepted Answer · 2014-02-24 16:10:48Z

A memory leak is when space is allocated on the heap (in your case by calling new) and the reference to that memory is lost. In other words, you have no way to reclaim the memory by calling delete. If you use swap to move pointers from one vector to another then technically that's not a leak because you still have a reference to the memory in the other vector and you can still call delete.

Remember to call delete eventually. It may be tempting in some cases depending on the use to just let the system cleanup later and not delete the memory ever, e.g. if the code is a CGI. However, that can cause problems later on when the code is used in a use case not initially anticipated, e.g. a unit test that's called from a long-running framework. It's generally worth the time to code the delete right away, even if doing so has no bearing on an immediate goal, rather than put yourself in the position where you might have to circle back and try to fix it later.

Collectives™ on Stack Overflow

Dealing with memory, when using std::vector

5 Answers 5

2 Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related