2010-03-03 3 views
1

Я только что начал комбинировать свои знания классов C++ и динамических массивов. Мне дали совет, что «всякий раз, когда я использую нового оператора», я должен удалить. Я также знаю, как работать деструкторов, так что я думаю, что этот код является правильным:Я использую здесь правильно удалить?

main.cpp

... 
int main() 
{ 
    PicLib *lib = new PicLib; 
    beginStorage(lib); 
    return 0; 
} 

void beginStorage(PicLib *lib) 
{ 
... 
    if (command != 'q') 
    { 
     //let's assume I add a whole bunch 
      //of stuff to PicLib and have some fun here 
     beginStorage(lib); 
    } 
    else 
    { 
     delete lib; 
     lib = NULL; 
     cout << "Ciao" << endl; 
    } 
} 

PicLib.cpp

... 

PicLib::PicLib() 
{ 
    database = new Pic[MAX_DATABASE]; 
    num_pics = 0; 
} 

PicLib::~PicLib() 
{ 
    delete[] database; 
    database = NULL; 
    num_pics = 0; 
} 
... 

Я наполняю свою PicLib с Pic класс, содержащий более динамические массивы. Дескриптор Pic удаляет их таким же образом, как показано выше. Я думаю, что delete [] database правильно избавляется от всех этих классов.

Так ли удаление в main.cpp необходимо? Здесь все выглядит здесь?

+0

Зачем выделять базу данных отдельно, а затем удалять ее? Если он всегда внутри класса, почему бы просто не использовать его как элемент Pic m_database [MAX_DATABASE]; – KPexEA

ответ

5

Есть несколько проблем:

int main() 
{ 
    PicLib *lib = new PicLib; 
    beginStorage(lib); 
    return 0; 
} 

Лучше выделить и удалить память в том же объеме, так что его легко обнаружить.

Но в этом случае просто объявить его локально (и передавать по ссылке):

int main() 
{ 
    PicLib lib; 
    beginStorage(lib); 
    return 0; 
} 

В beginStorage()

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

void beginStorage(PicLib& lib) 
{ 
.... 
} 

В классе PicLib у вас есть указатель RAW: базы данных.

Если у вас есть указатель RAW, который у вас есть (вы его создаете и уничтожаете), вы должны переопределить сгенерированные компилятором версии конструктора копирования и оператора присваивания. Но в данном случае я не вижу причин, touse указатель было бы проще просто использовать вектор:

class PivLib 
{ 
    private: 
     std::vector<Pic> databases; 
}; 
+0

Ах, извините за первый выпуск. Я заразил код. Упрощение трудно;) – Stephano

+0

Вот почему я люблю СО; вы, ребята, даете мне гораздо больше учиться! :п – Stephano

2

Да, это необходимо, если вы не используете auto_ptr (и прочитайте семантику auto_ptr перед его использованием - вы не можете скопировать его arround).

, например:

int main() 
{ 
    auto_ptr<PicLib> lib = new PicLib; 
    beginStorage(lib); 
    return 0; 
} // auto_ptr goes out of scope and cleans up for you 
0

Все выглядит хорошо.

Удалить в main.cpp необходимо, потому что если вы не вызывали delete, деструктор не запускается и ваш массив не удаляется. Вы также будете пропускать память для класса, а не только массива.

2

else не идет с while. Вы бы хотели что-то подобное:

void beginStorage(PicLib *lib) 
{ 
    while (command != 'q') 
    { 
     //let's assume I add a whole bunch 
      //of stuff to PicLib and have some fun here 
    } 

    delete lib; 
    lib = NULL; // Setting to NULL is not necessary in this case, 
       // you're changing a local variable that is about 
       // to go out of scope. 
    cout << "Ciao" << endl; 
} 

delete выглядит хорошо, но вы должны убедиться, что документ beginStorage становится владельцем объекта PicLib. Таким образом, любой, кто использует beginStorage, знает, что им не нужно его удалять позже.

+0

ops, извините. Я добавил if. я упрощал код и явно пропустил это :). Приветствия. – Stephano

3

Да, все, что вы создаете с новыйдолжны быть удалены с удалить, и все, что создано с новый []должен быть удален с удалить [], в какой-то момент.

Есть некоторые вещи, которые я укажу в вашем коде, хотя:

  • Предпочитают зЬй :: вектор <> по сравнению с использованием новых [] и удалите []. Он будет управлять памятью для вас. Также посмотрите на интеллектуальные указатели, такие как auto_ptr, shared_ptr и в C++ 0x unique_ptr для автоматического управления памятью. Это поможет вам забыть об удалении и вызвать утечку памяти. Если вы можете, не используйте новый/новый []/delete/delete [] вообще!
  • Если вам нужно что-то новое и что-то удалить, это очень хорошая идея для нового в конструкторе класса и удаления в деструкторе класса. Когда выбрасываются исключения, объекты выходят за пределы области видимости и т. Д., Их деструкторы вызываются автоматически, поэтому это помогает предотвратить утечку памяти. Это называется RAII.
  • Имею beginStorage удалить его параметр - потенциально плохая идея. Это может привести к сбою, если вы вызываете функцию с указателем, который не создан с новыми, потому что вы не можете удалить какой-либо указатель. Было бы лучше, если бы main() создал PicLib в стеке вместо того, чтобы использовать новые и удалять, и для beginStorage взять ссылку и ничего не удалить.
+1

Есть странность: 'typedef int a [1]; a * p = new a; delete [] p; '. Я бы сказал: «Если это массив, используйте delete []. Если это не так, используйте delete». –

+0

Ничего себе, это странный край. Не знал этого! – AshleysBrain

+0

Это странный случай края, но новый [] все еще используется, просто «скрыт» в typedef. – 2010-03-03 17:32:30

1

Обычно вы хотите удалить то же место, что и вы. Это упрощает учет. Лучше использовать интеллектуальный указатель (scoped_ptr в этом случае), что означает, что ваш код по-прежнему корректен, даже если тело while выбрасывает исключение и заканчивается преждевременно.

2

удалить в main.cpp необходимо.

Это, вероятно, относится к личным предпочтениям, но я бы посоветовал не называть новые и удалять в отдельных логических частях (здесь вызов delete в экземпляре PicLib находится в отдельной функции). Обычно лучше брать на себя ответственность за выделение и освобождение от ответственности только одной части.

@AshleysBrain предлагает лучшее предложение (о создании PicLib стека), хотя это может вызвать проблемы, если PicLib занимает слишком много памяти.

0

Да, в противном случае деструктор PicLib не будет вызван.

Одно стилистическое примечание: если вы новичок в объекте метода в функции, попробуйте и удалите его в той же функции. Будучи младшим инженером, которому пришлось пройти через крупные проекты, исправляющие утечки памяти других людей ... это облегчает чтение другим людям. По внешнему виду вы можете удалить * lib после того, как beginStorage() вернется. Тогда было бы легче увидеть объем * lib в одном месте.