2010-03-06 2 views
7

Я пытаюсь подключить все утечки памяти (что является массовым). Я новичок в STL. У меня есть библиотека классов, где у меня есть 3 набора. Я также создаю много памяти с новыми в классе библиотеки для добавления информации к наборам ...Утечки памяти - наборы STL

Нужно ли мне освобождать комплекты? Если да, то как?

Вот 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(); 
}; 

Я не уверен, что мне нужно сделать для деструктора?

Library::~Library() 
{ 


} 

также, я не выделяю stringset правильно?

#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; 

} 

в классе библиотеки я создаю много памяти, но не деструктор убрать это выдумал? пример:

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); 

Примечание: Я не конструктор копирования. Я не уверен, что мне даже нужен один или как добавить его. Я не думаю, что мои деструкторы вызваны либо.

+0

Краткий ответ: найдите каждый экземпляр символа '*' в своем коде и удалите его. Вам вряд ли когда-либо понадобится использовать указатели на C++. Каждый раз, когда вы это делаете, вы просите утечки памяти. – jalf

+4

Существует много раз, когда вам нужно использовать указатели на C++, даже если вы используете стандартные типы. – Xorlev

+0

В этом случае 'Item' является базовым классом для сохраненных объектов, поэтому вам нужно хранить указатели в' ItemSet' и 'ItemMap'. Хранение интеллектуальных указателей устраняет эти утечки. 'ItemSetMap' должен содержать только заданные объекты. –

ответ

3

Вам необходимо освободить память для каждого элемента набора. Контейнер не сделает этого для вас, и он не должен, потому что он не может знать, владеет ли он этими данными или нет - он может просто содержать указатели на объекты, принадлежащие чему-то другому.

Это бесплатная бесплатная функция, которая освободит любой контейнер 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

Но для этого вам нужен RTTI, ха! – pajton

+0

Зачем вам нужен RTTI? –

+0

Вероятно, для классов нужны виртуальные деструкторы. В любом случае, большинству классов нужны виртуальные деструкторы. Проблема - это свойство класса, а не функция deallocate_container, IOW. Одна проблема - работает ли она на std :: map, у которой есть ключ и данные для каждого элемента, или оба потенциально могут быть указателями? – Steve314

1

не прошли весь ваш код, но из первых нескольких строк кажется, что вы поддерживаете множество указателей. Всякий раз, когда у вас есть контейнер STL, который содержит указатели, и вы используете new, чтобы поместить вещи в указатели, вы должны использовать delete для освобождения этих указателей. STL не делает этого для вас. На самом деле, STL даже не знает, что они указатели.

Другой вариант - не использовать указатели вообще и иметь набор только объектов, а не использовать new для их создания. Просто создайте их в стеке и скопируйте их в набор.

0

В деструкторе вам нужно перебрать ваши коллекции stl, содержащие указатели и удалить их. Например:

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

Ну, это может быть глупый комментарий, но вы действительно должны иметь все ваши вещи в куче выделяется (иначе. Используя указатели и новые?)

Разве вы не можете просто использовать простые экземпляры ? RAII обеспечивает более легкий код и отсутствие утечки памяти.

Например есть:

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(); 
}; 

При таком подходе, деструктор имеет не делать совершенно ничего, и никакой утечки памяти. В большинстве случаев использование указателей можно легко избежать и, безусловно, должно!

0

Как уже отмечалось, вам необходимо освободить указатели. Обычно деструктор set не сделал этого для вас. В противном случае, если вы хотите, чтобы это было сделано для вас, используйте boost::scoped_ptr или std::tr1::shared_ptr, где вы можете указать пользовательский отладчик для выполнения этой задачи.

4

Контейнеры STL не предназначены для удержания указателей.

Посмотрите на контейнеры указателя повышения. Эти контейнеры предназначены для хранения указателей.

#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

Контейнеры проводить и собственные указатели, так что они будут удалены, когда контейнер выходит из области видимости. Но прекрасная вещь о контейнерах заключается в том, что вы обращаетесь к объектам через ссылки, поэтому все стандартные алгоритмы работают без каких-либо специальных адаптеров.

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

PS. Его трудно сказать точно, но похоже, что слишком много интерфейсов обратных указателей. В C++ очень редко на самом деле возвращать указатели (или пропустить указатели). Обычно ваши интерфейсы должны принимать объекты/ссылки или интеллектуальные указатели (обычно в этом порядке, но это зависит от ситуации).

Работа с указателем должна быть вашим последним средством, поскольку нет четкого указания владельца объекта, и поэтому очистка становится проблемой (что приводит к массивным утечкам памяти).

+0

+1 especialy о указателях в последней инстанции –

1

Рассматривая некоторые из кода, который вы отправили в других вопросах (например, https://stackoverflow.com/questions/2376099/c-add-to-stl-set), ваши товары хранятся в нескольких глобальных объектах ItemSet. Это не очень хороший дизайн - они действительно должны быть частью объекта Library, поскольку они логически принадлежат этому.

Лучший способ исправить утечки памяти - не иметь дело с необработанными указателями - либо хранить интеллектуальные указатели в наборах, либо использовать контейнеры указателей Boost, как предлагает Мартин Йорк. Кроме того, ваши объекты ItemSetMap должны содержать Set объекты, а не указатели - в них нет никаких оснований хранить указатели.

Если вы действительно должны хранить указатели, то ваш деструктор должен пройти через каждый набор для удаления содержимого:

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 
} 

, но это на самом деле не так, как вы должны делать это, как и почти все, отвечая на ваш вопросы.

Что касается StringSet, вы создали его с простой new не new[], так что вы должны удалить его с простым delete не delete[]. Или, еще лучше, сделайте объект music a StringSet, а не указатель, тогда вам вообще не понадобится деструктор. Еще раз, управление памятью с помощью необработанных указателей и ручное использование delete является подверженным ошибкам, и его следует избегать, если это вообще возможно.

+0

У меня все еще есть утечки памяти .. – 2010-03-07 02:35:59

+0

У меня также есть typedef set \t StringSet; StringSet * Moviecast; Могу я просто использовать delete Moviecast? или мне нужно перебирать? – 2010-03-07 02:43:29

+0

Нет, вам не нужно итерации 'set ', так как это содержит объекты и освободит их самостоятельно. Вам нужно только удалить объекты, которые вы выделили, используя 'new', и вы должны избегать этого, когда можете. –

Смежные вопросы