» Forum » Programowanie » [C, C++] Szukam pomocy

Poprawne usuwanie pamięci przydzielonej dynamicznie <new>.

Ostatnio zmodyfikowano 2017-09-23 12:08

0x07

Temat założony przez niniejszego użytkownika

» 2017-08-14 16:09:34

Cześć.

Wykonywałem ostatnio zadania z książki w ramach samodzielnej nauki c++. Zadanie polegało na stworzeniu szablonu opisującego kolejkę (strukturę danych rzecz jasna) i na jego podstawie zdefiniowaniu klasy Kolejki wskaźników na inną klasę (klasa ta nie obsługuje dynamicznego przydziału pamięci). Teoretycznie poradziłem sobie z tym zadaniem, ale dręczą mnie wątpliwości co do poprawnego usuwania pamięci przydzielonej dynamicznie:

int main()
{
using std::cin;
using std::cout;
using std::endl;
using std::strchr;

Kolejka < Worker *> queue( 3 );
Worker * pw[ 10 ];
menu();
int wybor;
int i = 0;
int del_licz = 0;

while( cin >> wybor && wybor < 5 )
{
switch( wybor )
{
case 1:
if( queue.isfull() )
   cout << "Kolejka pelna!\n";
else
{
pw[ i ] = new Waiter;
pw[ i ]->Set();
pw[ i ]->Show();
queue.enqueue( pw[ i ] );
cout << "Obiekt dodany pomyslnie. Obiektow w kolejce: " << queue.status() << '\n';
i++, del_licz++;
}
break;
case 2:
if( queue.isfull() )
   cout << "Kolejka pelna!\n";
else
{
pw[ i ] = new Singer;
pw[ i ]->Set();
pw[ i ]->Show();
queue.enqueue( pw[ i ] );
cout << "Obiekt dodany pomyslnie. Obiektow w kolejce: " << queue.status() << '\n';
i++, del_licz++;
}
break;
case 3:
if( queue.isfull() )
   cout << "Kolejka pelna!\n";
else
{
pw[ i ] = new SingingWaiter;
pw[ i ]->Set();
pw[ i ]->Show();
queue.enqueue( pw[ i ] );
cout << "Obiekt dodany pomyslnie. Obiektow w kolejce: " << queue.status() << '\n';
i++, del_licz++;
}
break;
case 4:
if( queue.isempty() )
   cout << "Kolejka pusta!\n";
else
{
cout << "Obiekt:\n";
queue.dequeue( pw[ --i ] );
pw[ i ]->Show();
cout << "Zdjety pomyslnie. Obiektow w kolejce: " << queue.status() << '\n';
}
break;
}
cout << "Wybierz 1, 2, 3, 4 lub 5: ";
}

int j = 0;
while( j < del_licz )
{
delete pw[ j ];
j++;
}
cout << "Koniec.\n";
return 0;
}

A tutaj szablon klasy:

#ifndef KOLEJKATP_H_
#define KOLEJKATP_H_

template < typename T >
class Kolejka
{
private:
enum { MAX = 10 };
struct Node { T item; struct Node * next; };
Node * front;
Node * rear;
int count;
int const rozm;
public:
explicit Kolejka( int roz = MAX );
~Kolejka();
bool isfull() const { return count == rozm; }
bool isempty() const { return count == 0; }
bool enqueue( T const & );
bool dequeue( T & );
int const status() const { return count; }
};

#endif // !KOLEJKATP_H_

template < typename T >
Kolejka < T >::Kolejka( int roz )
: rozm( roz )
{
front = rear = nullptr;
count = 0;
}

template < typename T >
Kolejka < T >::~Kolejka()
{
Node * temp;
while( front != nullptr )
{
temp = front;
front = front->next;
delete temp;
}
}

template < typename T >
bool Kolejka < T >::enqueue( T const & obj )
{
if( isfull() )
   return false;

Node * add = new Node;
add->item = obj;
count++;
add->next = nullptr;
if( front == nullptr )
   front = add;
else
   rear->next = add;

rear = add;
return true;
}

template < typename T >
bool Kolejka < T >::dequeue( T & obj )
{
if( front == nullptr )
   return false;

obj = front->item;
count--;
Node * temp = front;
front = front->next;
delete temp;
if( count == 0 )
   rear = nullptr;

return false;
}

W procesie debugowania kompilator pokazuje, że pamięć, na którą wskazują wskaźniki pw zostaje usunięta w pętli poprzedzającej instrukcję return main(). Chciałbym jednak poznać opinię kogoś bardziej doświadczonego (ode mnie), na temat poprawności usuwania pamięci przydzielonej dynamicznie w wyżej zamieszczonym programie. Jestem także otwarty na rady dotyczące usprawnienia usuwania pamięci <new> (jeszcze nie doszedłem do tematu inteligentnych wskaźników), które mogłyby mi oszczędzić trosk w przyszłości. Dziękuję z góry za zainteresowanie tematem.

Pozdrawiam serdecznie
0x07

P-163968

f651144

» 2017-09-23 12:08:42

Kiedy tak na szybko rzuciłem okiem, to wydaje mi się, że jest ok (zakładające przebieg iteracyjny).

Jednak, gdy konstruktor kopiujący rzuci wyjątek, problem pojawi się tu:

Node * add = new Node;
add->item = obj; // throw
count++;

a gdy rzuci destruktor, to tu będzie mały problem ze wskaźnikiem

rear

front = front->next;
delete temp; // throw
if( count == 0 )
rear = nullptr;

O rzucających destruktorach.

Co do usprawnienia usuwania pamięci - cytat z przytoczonego artykułu:

Of course, we all know that calling delete in our code is too low-level and typically indicates poor design and memory leaks(...)

Radziłbym zastosowanie inteligentnych wskaźników. Klasa

std::unique_ptr

wykona tu dokładnie to, co trzeba i to bez dodatkowego narzutu obliczeniowego. Twoja klasa wyglądała by tak:

#ifndef KOLEJKA_HPP_
#define KOLEJKA_HPP_

#include <memory>
#include <utility>

template < typename T >
class Kolejka
{
public:
explicit Kolejka( size_t roz = Kolejka < T >::limit_ )
: rear_( & front_ )
   , rozm_( roz )
{ };

Kolejka( const Kolejka < T >& ) = delete;

Kolejka( Kolejka < T >&& ) = default;

Kolejka < T >& operator =( const Kolejka < T >& ) = delete;

Kolejka < T >& operator =( Kolejka < T >&& ) = default;

~Kolejka()
{
// Zapobieganie przeładowaniu stosu ramkami destruktorów std::unique_ptr
while( this->front_ != nullptr )
   std::exchange( this->front_, std::move( this->front_->next ) );

};

bool full() const
{ return this->count_ == this->rozm_; };

bool empty() const
{ return this->count_ == 0; };

size_t max_size() const
{ return this->rozm_; };

size_t size() const
{ return this->count_; };

bool enqueue( const T & obj )
{
if( this->count_ == this->rozm_ )
   return false;

* this->rear_ = std::make_unique < Node_ >( obj );
this->rear_ = &( this->rear_->get()->next );

this->count_++;
return true;
};

bool dequeue( T & obj )
{
if( this->front_ == nullptr )
   return false;

obj = std::move( this->front_->item );

this->count_--;
if( this->count_ == 0 )
   this->rear_ = & this->front_;

this->front_ = std::move( this->front_->next );

return true;
};

private:
struct Node_
{
T item;
std::unique_ptr < Node_ > next = nullptr;

Node_( const T & obj )
: item( obj )
{ };

Node_( const Node_ & ) = delete;

Node_( Node_ && ) = default;

Node_ & operator =( const Node_ & ) = delete;

Node_ & operator =( Node_ && ) = default;

~Node_() = default;
};

static inline constexpr size_t limit_ = 10;

std::unique_ptr < Node_ > front_ = nullptr;
std::unique_ptr < Node_ >* rear_ = nullptr; // triple ref method

size_t count_ = 0;
const size_t rozm_ = 0;
};

#endif // !KOLEJKA_HPP_

Z pozostałych rzeczy wskazałbym, że interfejs klasy

Kolejka

nie jest najlepszy (może to tylko mój punkt widzenia):

Budując własne kontenery warto (jak najbardziej to potrzebne) upodobnić je do kontenerów z STL. Dla Twojego przypadku punktem odniesienia jest klasa
std::list
.
Nie do końa uważam za dobre zwracanie kopi obiektu przez metodę
bool dequeue( T & )
.
Wartości logiczne (
bool
) przy metodach
bool enqueue( T const & )
i
bool dequeue( T & )
też wydają się zbędne.
Do rozmiaru radziłbym stosować typ
size_t
(zamiast
int
).
W wielu miejscach można zastosować różne algorytmy z STL, które zwiększą czytelność i zmniejszą rozmiar kodu. Dla przykładu można zamienić
C/C++
Node * temp = front;
front = front->next;
delete temp;
na jedną linię
C/C++
delete std::exchange( front, front->next );

Powodzenia w dalszej nauce!

---------------

PS: Taka drobna uwaga - makra wokół treści plików nagłówkowych (na ogół) dobiera się na podstawie nazwy pliku, nie zaś typów w nim zawartych, tj.

#define KOLEJKA_H_

zamiast

#define KOLEJKATP_H_

. Ponadto pliki nagłówkowe języka C++ często zapisuje się z rozszerzeniem "hpp" (dla odróżnienia od plików nagłówkowych języka C, z rozszerzeniem "h"). Co jest jednak ważniejsze dyrektywa

#endif

powinna znajdować się na końcu pliku nagłówkowego.

PS2: Na końcu funkcji

bool dequeue( T & )

miało chyba być

return true

zamiast

return false

P-165147

« 1 »

Strona 1 z 1

» Forum » Programowanie » [C, C++] Szukam pomocy