Speicherverwalten bei Strategy Pattern. Gute Idee?

sirchillalot

Hiho alle zusammen,

schreibe gerade an einer Klasse bei der ich das Strategie Patter anwende.

Um das aktuelle Verhalten zu setzen biete ich eine setMethode an

void MeineKlasse::setStrategie(Strategie * stra);
{
   strategie = stra;
}

von außen kann ich also folgendes machen

MeinObjekt.setStrategie(new Strategie1());

Da ich hier allerding keinen Zugriff mehr habe auf das Objekt habe ich ja ein Memoryleak gebaut!?!

Nun zu meiner Frage. Ist es eine gute / Ausreichende Idee vor dem setzen der Strategie die alte zu deleten?

void MeineKlasse::setStrategie(Strategie * stra);
{
   delete strategie;
   strategie = stra;
}

Im Konstruktor habe ich die Strategie per default auf 0 gesetzt. Und im Konstruktor, delete ich die Strategie auch wieder.

DocShoe

Nimm shared_ptr aus der boost Bibliothek oder TR1, damit hast du alle Probleme gelöst.

sirchillalot

DocShoe schrieb:

Nimm shared_ptr aus der boost Bibliothek oder TR1, damit hast du alle Probleme gelöst.

habe hier leider kein boost zur Verfügung

Fellhuhn

Deine Klassen können sich sonst ja auch bei den Strategien an- und abmelden, so daß diese sich dann selbst zerstören.

sirchillalot

Fellhuhn schrieb:

Deine Klassen können sich sonst ja auch bei den Strategien an- und abmelden, so daß diese sich dann selbst zerstören.

Weiß gerade nicht was du meinst

Nexus

Du musst eine grundsätzliche Designentscheidung treffen. Nämlich solltest du festlegen, ob deine Strategy-Klasse ihre Strategien besitzt oder nur referenziert. Im ersten Fall ist sie selbst für die Freigabe verantwortlich, im zweiten wird der Speicher unabhängig davon irgendwo ausserhalb verwaltet. Der zweite Weg hat den Vorteil, dass dem Benutzer mehr Freiheiten gelassen werden (z.B. können automatische Objekten eingesetzt werden, und man hat mehr Kontrolle über die Erzeugung/Zerstörung seiner Objekte). Hier ein kleines Beispiel; wahrscheinlicher ist aber, dass es sich bei s um einen Member handelt (es sei denn, du willst die Strategie nur innerhalb des Scopes).

CustomStrategy s; // CustomStrategy erbt von Strategy
MyObject.setStrategy(&s);

Andererseits möchtest du das Strategie-Objekt vielleicht nicht ausserhalb speichern, sondern nach dem Zuweisen vergessen. Es besteht nämlich auch die Gefahr, dass der Zeiger ungültig wird, wenn das Objekt ausserhalb zerstört wird. Dann lohnt es sich, wenn das dynamisch angeforderte Strategy-Objekt von der Klasse selbst verwaltet und bei Bedarf auch zerstört wird. Denke dran, in diesem Fall die Grossen Drei (Kopierkonstruktor, Zuweisungsoperator, Destruktor) zu implementieren und dem Benutzer per Dokumentation genau mitzuteilen, dass er den Besitz über das Objekt verliert und dass es sich um mit new angeforderten Speicher handeln muss. Unmissverständlich gehts über std::auto_ptr .

MyObject.setStrategy(new CustomStrategy());

P.S.: shared_ptr gibt es wie gesagt auch im TR1, und der wird von vielen modernen Standardbibliotheken implementiert.

sirchillalot

Nexus schrieb:

Du musst eine grundsätzliche Designentscheidung treffen. Nämlich solltest du festlegen, ob deine Strategy-Klasse ihre Strategien besitzt oder nur referenziert. Im ersten Fall ist sie selbst für die Freigabe verantwortlich, im zweiten wird der Speicher unabhängig davon irgendwo ausserhalb verwaltet. Der zweite Weg hat den Vorteil, dass dem Benutzer mehr Freiheiten gelassen werden (z.B. können automatische Objekten eingesetzt werden, und man hat mehr Kontrolle über die Erzeugung/Zerstörung seiner Objekte). Hier ein kleines Beispiel; wahrscheinlicher ist aber, dass es sich bei s um einen Member handelt (es sei denn, du willst die Strategie nur innerhalb des Scopes).
CustomStrategy s; // CustomStrategy erbt von Strategy
MyObject.setStrategy(&s);
Andererseits möchtest du das Strategie-Objekt vielleicht nicht ausserhalb speichern, sondern nach dem Zuweisen vergessen. Es besteht nämlich auch die Gefahr, dass der Zeiger ungültig wird, wenn das Objekt ausserhalb zerstört wird. Dann lohnt es sich, wenn das dynamisch angeforderte Strategy-Objekt von der Klasse selbst verwaltet und bei Bedarf auch zerstört wird. Denke dran, in diesem Fall die Grossen Drei (Kopierkonstruktor, Zuweisungsoperator, Destruktor) zu implementieren und dem Benutzer per Dokumentation genau mitzuteilen, dass er den Besitz über das Objekt verliert und dass es sich um mit new angeforderten Speicher handeln muss. Unmissverständlich gehts über std::auto_ptr .
MyObject.setStrategy(new CustomStrategy());
P.S.: shared_ptr gibt es wie gesagt auch im TR1, und der wird von vielen modernen Standardbibliotheken implementiert.

Punkt 2 ist das was ich möchte. Der User soll nur ein "anonymes" Objekt an die Mehtode übergeben. Um das löschen soll sich die Strategieklasse kümmern.
Ich versuche gerade boost unter Borland c++ Builder 2006 zu installieren. Scheint aber nicht ganz einfach zu sein.

DocShoe

Eine weitere Möglichkeit wäre eine clone() Funktion für alle Strategy Klassen zum implementieren, sodass der Client per clone() eine private Kopie erzeugen kann, die er auch besitzt. Mit dieser Technik können sogar Clients kopiert werden, ohne dass es zu Konflikten kommt:

class MyObject
{
   Strategy* Strategy_;

public:
   MyObject() : Strategy_( 0 )
   {
   }

   MyObject( const MyObject& obj ) : Stratey_( 0 )
   {
      set_strategy( obj->Strategy_  );
   }

   MyObject& operator=( const MyObject& obj )
   {
      if( this != &obj )
      {
         set_strategy( obj->Strategy_  );
      }
      return *this;
   }

   ~MyObject()
   {
      set_strategy( 0 );
   }

   void set_strategy( const Strategy* Strategy )
   {
      Strategy* Clone = 0;
      if( Strategy )
      {
         Clone = Strategy->clone();
      }
      if( Strategy_ )
      {
         delete Strategy_;
      }
      Strategy_ = Clone; 
   }
};

DocShoe

Ich benutze Codegear RAD Studio 2007 und die letzte funktionierende boost Bibliothek ist 1.34.1. Die Versionen 1.40 und 1.41 habe ich noch nicht getestet, aber 1.36 und 1.39 konnte ich nicht kompilieren, zumindest nicht alle Bibliotheken.
smart_ptr.hpp sollten aber alle gehen, da es eine Header-only Bibliothek ist.

Jockelx

Warum nicht einfach den Copy-Ctor statt dieser Clone-Funktion?

DocShoe

Jockelx schrieb:

Warum nicht einfach den Copy-Ctor statt dieser Clone-Funktion?

Weil das Strategy Objekt polymorph und der konkrete Typ nicht bekannt ist.