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



  • 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'?



  • 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.



  • @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.



  • 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()

  • Lifetime Qt Champion

    Hi,

    Or use qDeleteAll and then clear your container.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.