2010-03-06 16 views
7

Estoy tratando de tapar todas las fugas de memoria (que es masivo). Soy nuevo en STL. Tengo una biblioteca de clase donde tengo 3 juegos. También estoy creando mucha memoria con los nuevos en la clase de la biblioteca para agregar información a los conjuntos ...Fugas de memoria - conjuntos de STL

¿Debo desasignar los juegos? ¿Si es así, cómo?

Aquí es el library.h

#pragma once 

#include <ostream> 
#include <map> 
#include <set> 
#include <string> 
#include "Item.h" 

using namespace std; 

typedef set<Item*>    ItemSet; 
typedef map<string,Item*>  ItemMap; 
typedef map<string,ItemSet*> ItemSetMap; 

class Library 
{ 

public: 
    // general functions 

    void addKeywordForItem(const Item* const item, const string& keyword); 
    const ItemSet* itemsForKeyword(const string& keyword) const; 
    void printItem(ostream& out, const Item* const item) const; 

    // book-related functions 

    const Item* addBook(const string& title, const string& author, int const nPages); 
    const ItemSet* booksByAuthor(const string& author) const; 
    const ItemSet* books() const; 

    // music-related functions 

    const Item* addMusicCD(const string& title, const string& band, const int nSongs); 
    void addBandMember(const Item* const musicCD, const string& member); 
    const ItemSet* musicByBand(const string& band) const; 
    const ItemSet* musicByMusician(const string& musician) const; 
    const ItemSet* musicCDs() const; 

    // movie-related functions 

    const Item* addMovieDVD(const string& title, const string& director, const int nScenes); 
    void addCastMember(const Item* const movie, const string& member); 
    const ItemSet* moviesByDirector(const string& director) const; 
    const ItemSet* moviesByActor(const string& actor) const; 
    const ItemSet* movies() const; 
    ~Library(); 
}; 

No estoy seguro de lo que tengo que hacer por el destructor?

Library::~Library() 
{ 


} 

también, ¿estoy de allocating the stringset right?

#ifndef CD_H 
#define CD_H 
#pragma once 
#include "item.h" 
#include <set> 


typedef set<string> StringSet; 


class CD : public Item 
{ 
public: 

    CD(const string& theTitle, const string& theBand, const int snumber); 
    void addBandMember(const string& member); 
    const int getNumber() const; 
    const StringSet* getMusician() const; 
    const string getBand() const; 
    virtual void print(ostream& out) const; 
    string printmusicians(const StringSet* musicians) const; 

    ~CD(); 


private: 

    string band; 
    StringSet* music; 

    string title; 
    int number; 

}; 

ostream& operator<<(ostream& out, const CD* cd); 

#endif 

cd.cpp

#include "CD.h" 

using namespace std; 

CD::CD(const string& theTitle, const string& theBand, const int snumber) 
: Item(theTitle), band(theBand),number(snumber), music(new StringSet) 
{ 



} 

CD::~CD() 
{ 

    delete []music; 

} 

en la biblioteca de clases que estoy creando una gran cantidad de memoria, pero no te el destructor limpiar eso? ejemplo:

const Item* Library::addBook(const string& title, const string& author, const int nPages) 
{ 

    ItemSet* obj = new ItemSet(); 
    Book* item = new Book(title,author,nPages); 
    allBooks.insert(item); // add to set of all books 
    obj->insert(item); 

Nota: No tengo un constructor de copia. No estoy seguro si incluso necesito uno o cómo agregar uno. No creo que mis destructores sean llamados tampoco ...

+0

Respuesta breve: Busque todas las instancias del carácter '*' en su código y bórrelo. Casi nunca necesita utilizar punteros en C++. Cada vez que lo haces, estás pidiendo pérdidas de memoria. – jalf

+4

Hay muchas veces que necesita utilizar punteros en C++, incluso si usa tipos estándar. – Xorlev

+0

En este caso, 'Item' es una clase base para los objetos almacenados, por lo que debe almacenar los punteros en' ItemSet' y 'ItemMap'. El almacenamiento de punteros inteligentes solucionará esas filtraciones. 'ItemSetMap' solo debería contener objetos establecidos. –

Respuesta

3

Debe liberar la memoria para cada elemento del conjunto. El contenedor no lo hará por usted, y no debería hacerlo, ya que no puede saber si posee esos datos o no; podría contener punteros a objetos propiedad de otra cosa.

Esta es una función gratuita genérica que desasignará cualquier contenedor STL.

template <typename T> 
void deallocate_container(T& c) 
{ 
    for (typename T::iterator i = c.begin(); i != c.end(); ++i) 
    delete *i; 
} 

// Usage 
set<SomeType*> my_set; 
deallocate_container(my_set); 
my_set.clear(); 
+0

Pero necesitas RTTI para eso, ¡ja! – pajton

+0

¿Por qué necesitaría RTTI para esto? –

+0

Probablemente necesita que las clases tengan destructores virtuales. La mayoría de las clases necesitan destructores virtuales de todos modos. El problema es una propiedad de la clase, no esta función deallocate_container, IOW. Sin embargo, hay un problema: ¿funciona para std :: map, que tiene clave y datos para cada elemento, uno o ambos potencialmente un puntero? – Steve314

1

no han pasado por todo su código pero desde las primeras líneas parece que están manteniendo conjuntos de punteros. Siempre que tenga un contenedor STL que contenga punteros y esté usando new para poner cosas en los punteros, debe usar delete para desasignar estos punteros. STL no hace eso por ti. De hecho, STL ni siquiera sabe que son punteros.

Otra opción es no utilizar punteros en absoluto y tener un conjunto de solo los objetos y no usar new para crearlos. Simplemente créelos en la pila y cópielos en el conjunto.

0

En el destructor necesita iterar sobre sus colecciones stl que contienen punteros y eliminarlos. De esta manera:

while (!collection.empty()) { 
    collection_type::iterator it = collection.begin(); 
    your_class* p = *it; 
    collection.erase(it); 
    delete p; 
} 
1

Bueno, esto podría ser un comentario estúpido, pero es lo que realmente necesita tener todas sus cosas montón asignados (aka. Punteros y utilizando de nuevo?)

¿No puedes utilizar instancias de civil ? RAII permite código más fácil y sin pérdida de memoria.

Por ejemplo tener:

using namespace std; 

typedef set<Item>    ItemSet; 
typedef map<string,Item>  ItemMap; 
typedef map<string,ItemSet> ItemSetMap; 

class Library 
{ 

public: 
    // general functions 

    void addKeywordForItem(const Item & item, const string& keyword); 
    ItemSet itemsForKeyword(const string& keyword) const; 
    void printItem(ostream& out, const Item & item) const; 

    // book-related functions 

    Item addBook(const string& title, const string& author, int nPages); 
    ItemSet booksByAuthor(const string& author) const; 
    ItemSet books() const; 

    // music-related functions 

    Item addMusicCD(const string& title, const string& band, int nSongs); 
    void addBandMember(const Item & musicCD, const string& member); 
    ItemSet musicByBand(const string& band) const; 
    ItemSet musicByMusician(const string& musician) const; 
    ItemSet musicCDs() const; 

    // movie-related functions 

    Item addMovieDVD(const string& title, const string& director, int nScenes); 
    void addCastMember(const Item & movie, const string& member); 
    ItemSet moviesByDirector(const string& director) const; 
    ItemSet moviesByActor(const string& actor) const; 
    ItemSet movies() const; 
    ~Library(); 
}; 

Con este enfoque, el destructor tiene que ver estrictamente nada, y sin pérdida de memoria. En la mayoría de los casos, el uso de punteros se puede evitar fácilmente, ¡y definitivamente debería ser así!

0

Como han señalado otros, debe desasignar los punteros. El destructor set normalmente no haría esto por usted. De lo contrario, si desea que esto se haga por usted, use un boost::scoped_ptr o un std::tr1::shared_ptr donde puede especificar un eliminador personalizado para hacer este trabajo por usted.

4

Los contenedores STL no están diseñados para contener punteros.

Mire los contenedores del indicador de refuerzo. Estos contenedores están diseñados para contener punteros.

#include <boost/ptr_container/ptr_set.hpp> 
#include <boost/ptr_container/ptr_map.hpp> 

http://www.boost.org/doc/libs/1_42_0/libs/ptr_container/doc/ptr_set.html

Los recipientes contienen y son dueños de los punteros para que se eliminarán cuando el contenedor sale del ámbito. Pero lo bonito de los contenedores es que accedes a los objetos a través de referencias para que todos los algoritmos estándar funcionen sin adaptadores especiales.

typedef boost::ptr_set<Item>    ItemSet; 
typedef boost::ptr_map<string,Item>  ItemMap; 
typedef boost::ptr_map<string,ItemSet> ItemSetMap; 

PS. Es difícil de decir con precisión, pero parece que muchas de sus interfaces devuelven punteros. En C++ es muy raro devolver los punteros (o pasar punteros). Sus interfaces generalmente deben tomar objetos/referencias o punteros inteligentes (generalmente en ese orden pero depende de la situación).

Trabajar con un puntero debería ser su último recurso ya que no hay una indicación clara del propietario del objeto y, por lo tanto, la limpieza se convierte en un problema (lo que provoca fugas masivas de memoria).

+0

+1 especialmente sobre punteros siendo el último recurso –

1

Mirando parte del código que publicó en otras preguntas (como https://stackoverflow.com/questions/2376099/c-add-to-stl-set), sus artículos se almacenan en varios objetos globales ItemSet. Este no es un buen diseño: deberían ser parte del objeto Library, ya que lógicamente pertenecen a ese.

La mejor manera de solucionar las fugas de memoria es no tratar con punteros crudos, ya sea almacenar punteros inteligentes en los conjuntos, o usar contenedores de puntero Boost como sugiere Martin York. Además, sus objetos ItemSetMap deben contener Set objetos en lugar de punteros; no hay absolutamente ninguna razón para almacenar punteros en ellos.

Si realmente tiene que almacenar punteros, entonces su destructor debe caminar a través de cada conjunto para eliminar el contenido:

void Purge(ItemSet &set) 
{ 
    for (ItemSet::iterator it = set.begin(); it != set.end(); ++it) 
     delete *it; 
    set.clear(); // since we won't actually be destroying the container 
} 

void Purge(ItemSetMap &map) 
{ 
    for (ItemSetMap::iterator it = map.begin(); it != map.end(); ++it) 
     delete it->second; 
    map.clear(); 
} 

Library::~Library() 
{ 
    Purge(allBooks); 
    Purge(allCDs); 
    // and so on and so forth 
} 

pero esto realmente no es la forma en que debe hacerlo, ya que casi todo el mundo respondiendo a su preguntas ha señalado.

En cuanto a la StringSet, lo creó con sencillo new no new[], por lo que hay que eliminarla con sencillo delete no delete[]. O, mejor aún, haga que music sea un objeto StringSet en lugar de un puntero, entonces no necesitará el destructor en absoluto. Una vez más, la administración de la memoria a través de punteros sin procesar y el uso manual de delete es propenso a errores, y debe evitarse si es posible.

+0

Todavía tengo muchas fugas de memoria ... –

+0

También tengo typedef set \t StringSet; StringSet * Moviecast; ¿Puedo usar eliminar Moviecast en él? o tengo que iterar? –

+0

No, no tiene que iterar un 'set ', ya que contiene objetos y los desasignará. Solo necesita borrar los objetos que asignó usando 'new', y debe evitar hacerlo cada vez que pueda. –