This seems redundant to me? Delete each single value of a map and then

JohnFrom

QMap<QString, QPixmap*>::const_iterator item;
   for( item = item_container.begin(); item !=  item_container.end(); item++){
       delete item.value();
   }
item_container.clear();

Doesn't clear() clears everything? Why does the code do it 'twice'?

6thC

QPixmap <pointer> - I'd say it's cleaning up allocated memory via a pointer.
delete is gracefully destroying the object at the memory address is item.value() gives.
clear is the elements / container cleanup.

J.Hilk

@JohnFrom the code doesn't do it twice.

You're maping QPixmaps that are created on the heap and therefore have to be deleted. Calling delete on the objects deletes it. But that leaves the QMap still intact, with pointers to memory, that is no longer valid.

clear only removes references / pointers from the QMap.

If you only call clear, the QMap is reset, but the memory is still occupied -> memory leak.

VRonin

2 side notes:

if the QMap owns those QPixmap then there's (close to) no advantage in storing it as QMap<QString, QPixmap*> (Qt internally optimises the processes so that it's only marginally worse than a raw pointer) only the burden of deleting the values manually.
Consider changing it to QMap<QString, QPixmap> if appropriate
do not mix const and non-const iterators. Either declare QMap<QString, QPixmap*>::iterator item; or use item_container.cbegin()/item_container.cend()

SGaist

Hi,

Or use qDeleteAll and then clear your container.