Kann ich sowas bedenkenlos schreiben?

starki

Hallo Leute,

eine kleine Frage: Ich habe eine Klasse:

class Punkt
{
private:
   std::vector<float> *coordinates;
public:
   Punkt(size_t numOfCoordinates);
   // ... weitere Funktionen ...
};

Punkt::Punkt(size_t numOfCoordinates)
{
   coordinates = new std::vector<float>();
}

Wenn ich jetzt in einer Funktion diese Klasse verwende, beispielsweise:

function generatePoints()
{
   Punkt p1(2);
   Punkt p2(2);
   // ... mach irgendwas mit den Punkten
}

Was passiert mit meinem Pointer coordinates? Bleibt er irgendwo im Speicher übrig, muss ich das selbst frei machen, macht das C++ automatisch oder was passiert mit dem Pointer und den Daten / Speicherplatz?

volkard

starki schrieb:

Was passiert mit meinem Pointer coordinates? Bleibt er irgendwo im Speicher übrig, muss ich das selbst frei machen, macht das C++ automatisch oder was passiert mit dem Pointer und den Daten / Speicherplatz?

Jup, Du hast das Problem erfaßt. Der bleibt im Speicher übrig. Du müßtest ihn freimachen, im Destruktor günstigerweise.

Vermutlich ist eh

class Punkt
{
private:
   std::vector<float> coordinates;
public:
   Punkt(size_t numOfCoordinates);
   // ... weitere Funktionen ...
};

passender.

Nathan

Der bleibt im Speicher übrig, denn zu jedem new gehört ein delete.
Aus diesem Grund nutzt man in C++ Destruktoren, um die Arbeiten in Konstruktoren rückgängig zu machen.
Dann braucht man aber auch Kopierkonstruktoren, Zuweisungsoperatoren und ggf. dasselbe nochmal mit move.
Deshalb vermeidet man rohes new und delete, sondern nutzt bessere Methoden.
In diesem Fall ist die bessere Methode einfach auf den Pointer zu verzichten und die Koordinaten als normalen Member in der Klasse zu haben.

daddy_felix

Was du mit new erzeugst, musst du mit delete zerstören (Ausnahme: Smart Pointer).

Warum soll der std::vector überhaupt ein Pointer sein?

DrakoXP

Das sollte wohl eher so aussehen:

class Punkt
{
private:
   std::vector<float> coordinates; // kein Zeiger
public:
   Punkt(size_t numOfCoordinates);
   // ... weitere Funktionen ...
};

Punkt::Punkt(size_t numOfCoordinates)
   : coordinates(numOfCoordinated, .0f) // Speicher reservieren und mit 0en füllen
{
}

DAS ist unbedenklich, da der Vektor seinen Speicher am Ende automatisch freigibt.
Deine Variante hat allerdings den Vektor selbst auch nochmal dynamisch angelegt und nirgends freigegeben.

icarus2

Du musst den Speicher explizit mit delete freigeben. Das sollte im Destruktor von Point gemacht werden:

~Punkt()
{
     delete coordinates;
}

Aber warum überhaupt einen Pointer auf std::vector ? Besser ist

class Punkt
{
private:
   std::vector<float> coordinates;
public:
   Punkt(size_t numOfCoordinates);
   // ... weitere Funktionen ...
};

Punkt::Punkt(size_t numOfCoordinates)
    : coordinates(numOfCoordinates)
{ }

Hier musst du den Speicher nicht explizit allozieren und freigeben.

icarus2 schrieb:

Du musst den Speicher explizit mit delete freigeben. Das sollte im Destruktor von Point gemacht werden:
~Punkt()
{
     delete coordinates;
}

Nein, das ist dann ganz falsch.
http://en.wikipedia.org/wiki/Rule_of_three

icarus2

ro3 schrieb:

icarus2 schrieb:
Du musst den Speicher explizit mit delete freigeben. Das sollte im Destruktor von Point gemacht werden:
~Punkt()
{
     delete coordinates;
}
Nein, das ist dann ganz falsch.
http://en.wikipedia.org/wiki/Rule_of_three

Na klar, man muss natürlich den Rest entsprechend anpassen (hat ja auch niemand etwas anderes gesagt). Deswegen ist es noch lange nicht falsch mit delete den Speicher im Destruktor freizugeben.

icarus2 schrieb:

ro3 schrieb:
icarus2 schrieb:
Du musst den Speicher explizit mit delete freigeben. Das sollte im Destruktor von Point gemacht werden:
~Punkt()
{
     delete coordinates;
}
Nein, das ist dann ganz falsch.
http://en.wikipedia.org/wiki/Rule_of_three
Na klar, man muss natürlich den Rest entsprechend anpassen (hat ja auch niemand etwas anderes gesagt). Deswegen ist es noch lange nicht falsch mit delete den Speicher im Destruktor freizugeben.

So wie ich diesen Typ TE hier kenne wird er deinen Code kopieren weil ihm das am naheliegendsten vorkommt. Dann wundert er sich wegen korruptem Speicher und fragt wieder nach.

Wenn, dann muss man ihm die ganze Ladung ins Gesicht blasen:

// Wer einen Destruktor hat ...
~Punkt() {
    delete coordinates;
}

// ... der muss auch einen Kopierkonstruktor ...
Punkt(Punkt const& o)
  : coordinates(o.coordinates) {}

// ... und den Zuweisungsoperator implementieren.
friend void swap(Punkt& a, Punkt& b) {
    std::swap(a.coordinates, b.coordinates);
}
Punkt& operator=(Punkt o) { // google: copy and swap idiom
    std::swap(*this, o);
    return *this;
}

ro3 schrieb:

Wenn, dann muss man ihm die ganze Ladung ins Gesicht blasen:

// Wer einen Destruktor hat ...
~Punkt() {
    delete coordinates;
}

// ... der muss auch einen Kopierkonstruktor ...
Punkt(Punkt const& o)
  : coordinates(o.coordinates) {}

// ... und den Zuweisungsoperator implementieren.
friend void swap(Punkt& a, Punkt& b) {
    std::swap(a.coordinates, b.coordinates);
}
Punkt& operator=(Punkt o) { // google: copy and swap idiom
    std::swap(*this, o);
    return *this;
}

Warum bläst du ihm eine fehlerhafte Ladung ins Gesicht?
Und es fehlt noch move für C++11 (wenn du ihm schon alles vorkauen willst).

Skym0sh0

ro3 schrieb:

Wenn, dann muss man ihm die ganze Ladung ins Gesicht blasen:

// Wer einen Destruktor hat ...
~Punkt() {
    delete coordinates;
}

// ... der muss auch einen Kopierkonstruktor ...
Punkt(Punkt const& o)
  : coordinates(o.coordinates) {}

// ... und den Zuweisungsoperator implementieren.
friend void swap(Punkt& a, Punkt& b) {
    std::swap(a.coordinates, b.coordinates);
}
Punkt& operator=(Punkt o) { // google: copy and swap idiom
    std::swap(*this, o);
    return *this;
}

Da sind die automatisch generierten Konstrukte schon besser, obwohl nur ein kleines Zeichen fehlt (Tipp: es sit ein Stern ;))

Belli

Wie willst Du denn den gezeigten Kopierkonstruktor mit einem Stern korrigieren?

Skym0sh0

Belli schrieb:

Wie willst Du denn den gezeigten Kopierkonstruktor mit einem Stern korrigieren?

Ups
Ein Fail nach dem anderen

Naja so hatte ich mir das gedacht...

// Wer einen Destruktor hat ...
~Punkt() {
    delete coordinates;
}

// ... der muss auch einen Kopierkonstruktor ...
Punkt(Punkt const& o)
  : *coordinates(*o.coordinates) {}

// ... und den Zuweisungsoperator implementieren.
friend void swap(Punkt& a, Punkt& b) {
    std::swap(a.coordinates, b.coordinates);
}
Punkt& operator=(Punkt o) { // google: copy and swap idiom
    std::swap(*this, o);
    return *this;
}

DocShoe

Ich könnte noch mal
Da fehlt dann noch ein new in Zeile 8:

Punkt( const Punkt& rhs ) :
   coordinates( new vector<float>( *rhs.coordinates ) )
{
}

Skym0sh0

Man sind wir hier pervers