Probleme mit dem Destructor



  • Ich habe noch sehr wenig Ahnung von c++ und habe versucht eine simple Vector classe zu schreiben. Beim Aufruf des Destrucktors wird das Programm mit dem
    Fehler: "CRT detected that the application wrote to memory after the end of heap buffer" beednet.

    Hier der code:

    Vector3D.h

    class Vector3D {
    public:
    
    	public:	Vector3D (double , double , double );
    	public: Vector3D ();
    
    	void operator + (Vector3D*); 
    	void operator * (int);
    	~Vector3D (void); 
    	void normalisieren ();
        void set_x (double ); 
        void set_y (double );
        void set_z (double );
    	double get_x ();
    	double get_y ();
    	double get_z ();
    
    };
    

    Vector3D.cpp

    #include <cmath>
    
    class Vector3D {
    
    private: double* x_;
    		 double* y_;
    		 double* z_;
    
    	public:	Vector3D (double , double , double );
    	public: Vector3D ();
    	public:	~Vector3D (void); 
    	public:void operator + (Vector3D *); 
    	public:void operator * (int x); 
    	public:void normalisieren ();
        public:void set_x (double ); 
        public:void set_y (double );
        public:void set_z (double );
    	public:double get_x ();
    	public:double get_y ();
    	public:double get_z ();
    
    };
    
    Vector3D::Vector3D (double x, double y, double z) 
    		{
    		x_ = new double (x);
    		y_ = new double (y);
    		z_ = new double (z);
    
    		}
    
    Vector3D::Vector3D () 
    		{
    		x_ = new double (0);
    		y_ = new double (0);
    		z_ = new double (0);
    
    		}
    
    Vector3D::~Vector3D (void) 
    		{
    //		delete x_;
    //		delete y_;
    //		delete z_;
    //egal ob diese 3 Zeilen auskommentiert sind oder nicht, es knallt immer
    
    		}
    
    void Vector3D::operator + (Vector3D* vec) 
    		{
    		*x_ = *x_ + *vec->x_;
    		*y_ = *y_ + *vec->y_;
    		*z_ = *z_ + *vec->z_;
    		}
    
    void Vector3D::operator * (int x) 
    		{
    		*x_=*x_ * x;
    		*y_=*y_ * x;
    		*z_=*z_ * x;
    
    		}
    
    void Vector3D::normalisieren () 
    		{
            long double laenge = sqrt (*x_ * *x_ + *y_ * *y_ + *z_ * *z_); 
    		if (laenge!=0) {
    		 *x_ = *x_ / laenge;
    		 *y_ = *x_ / laenge;
    		 *z_ = *x_ / laenge;
    		}
    
    		}
    
    void Vector3D::set_x (double x) 
    		{
    		*x_=x;
    		}
    
    void Vector3D::set_y (double y) 
    		{
    		*y_=y;
    		}
    
    void Vector3D::set_z (double z) 
    		{
    		*z_=z;
    		}
    
    double Vector3D::get_x ()
    		{
    		return *x_;
    		}
    
    double Vector3D::get_y ()
    		{
    		return *y_;
    		}
    
    double Vector3D::get_z ()
    		{
    		return *z_;
    		}
    

    test.cpp

    #include <iostream>
    #include "Vector3D.h"
    
    using namespace std;
    
    int main() {
    
    Vector3D* v1= new Vector3D(7,5,3);
    Vector3D* v2= new Vector3D(2,-6,20);
    //*v1+v2;
    
    //cout << v1->get_y();
    
    delete v1;   //Hier knallt es
    delete v2;
    
    return 1;
    
    }
    

    Was habe ich falsch gemacht?



  • Warum eigentlich pointer auf double? Ist nicht ganz so sinnig in der Vektor Klasse...



  • (D)Evil schrieb:

    Warum eigentlich pointer auf double? Ist nicht ganz so sinnig in der Vektor Klasse...

    Weil ich den Umgang mit Zeigern üben wollte und mir ist nix beseres als Projekt eingefallen.

    Aber wo ist denn der Fehler?



  • So ich habe es jetzt selber herausgefunden:

    Es fehlt in der HEADERDATEI in der Klasse:

    private: double* x_;
             double* y_;
             double* z_;
    

    Jetzt währe es noch schön, wenn mir jemand erklähren würde, warum das da hin muß.



  • In Deiner Implementierung ist vieles doppelt! Streichen. Ich habe das mal in eine Form gebracht, die zumindest bis zum Ende durchläuft und auch das richtige Ergebnis bringt.

    #include <cmath>
    #include <iostream>
    
    void wait() //ausführliche Version
    {
        std::cin.clear();
        std::streambuf* pbuf = std::cin.rdbuf();
        std::streamsize size = pbuf->in_avail();
        std::cin.ignore(size);
        std::cin.get();
    }
    
    class Vector3D 
    {
    public:
        Vector3D (double , double , double );
        Vector3D ();
        void operator + (Vector3D*);
        void operator * (int);
        ~Vector3D ();
        void normalisieren ();
        void set_x (double );
        void set_y (double );
        void set_z (double );
        double get_x ();
        double get_y ();
        double get_z ();
    private: 
        double* x_;
        double* y_;
        double* z_;
    };
    
    /************ Implementierung ************************/
    Vector3D::Vector3D (double x, double y, double z)
    {
        x_ = new double (x);
        y_ = new double (y);
        z_ = new double (z);
    }
    
    Vector3D::Vector3D ()
    {
        x_ = new double (0);
        y_ = new double (0);
        z_ = new double (0);
    }
    
    Vector3D::~Vector3D (void)
    {
        delete x_;
        delete y_;
        delete z_;
    }
    
    void Vector3D::operator + (Vector3D* vec)
    {
        *x_ = *x_ + *vec->x_;
        *y_ = *y_ + *vec->y_;
        *z_ = *z_ + *vec->z_;
    }
    
    void Vector3D::operator * (int x)
    {
        *x_=*x_ * x;
        *y_=*y_ * x;
        *z_=*z_ * x;
    }
    
    void Vector3D::normalisieren ()
    {
         long double laenge = sqrt (*x_ * *x_ + *y_ * *y_ + *z_ * *z_);
         if (laenge!=0) 
         {
             *x_ = *x_ / laenge;
             *y_ = *x_ / laenge;
             *z_ = *x_ / laenge;
         }
    }
    
    void Vector3D::set_x (double x)
    {
         *x_= x;
    }
    
    void Vector3D::set_y (double y)
    {
         *y_=y;
    }
    
    void Vector3D::set_z (double z)
    {
         *z_=z;
    }
    
    double Vector3D::get_x ()
    {
         return *x_;
    }
    
    double Vector3D::get_y ()
    {
         return *y_;
    }
    
    double Vector3D::get_z ()
    {
         return *z_;
    }
    /************ Implementierung ************************/
    
    int main() 
    {
        Vector3D* v1= new Vector3D(7,5,3);
        Vector3D* v2= new Vector3D(2,-6,20);
    
        std::cout << v1->get_y();
    
        delete v1;  
        delete v2;
        wait();
        return 0;
    }
    

    Schau Dir mal das Thema Initialisierungsliste beim Konstruktor an. Da kannst Du noch einiges verbessern. Insbesondere bei "const-correctness" hapert es ebenfalls noch. Schau Dir das bitte mal an, z.B. sind die get-Funktionen const!

    Ob man die Koordinaten auf dem Freispeicher anlegen sollte, darüber kann man streiten, aber was soll's. Verschiedene Denkansätze können nicht schaden. 😉



  • Jetzt währe es noch schön, wenn mir jemand erklähren würde, warum das da hin muß.

    Ja, warum müssen Attribute in einer Klasse sein? Diese drei Koordinaten sind doch der Kern Deiner Klasse! Alles dreht sich um diese Werte. Die Klasse "kapselt" diese drei Werte durch "private" vor dem Rest der Welt. Deine Verwirrung rührt daher, dass unten in der cpp-Datei zu viel ist. Alles irgendwie doppelt. Das packst Du doch mit "#include header" dazu.



  • Ich habe die Initialisierungs-Listen (siehe Scott Meyers, Effektiv C++ programmieren, Lektion 12) und die const (siehe Scott Meyers, Effektiv C++ programmieren, Lektion 21) dazu gepackt und bei den get-/set-Methoden inline (siehe Scott Meyers, Effektiv C++ programmieren, Lektion 33) verwendet.

    #include <cmath>
    #include <iostream>
    
    void wait() //ausführliche Version
    {
        std::cin.clear();
        std::streambuf* pbuf = std::cin.rdbuf();
        std::streamsize size = pbuf->in_avail();
        std::cin.ignore(size);
        std::cin.get();
    }
    
    class Vector3D 
    {
    public:
        Vector3D (double , double , double );
        Vector3D ();
        void operator + (Vector3D*);
        void operator * (int);
        ~Vector3D ();
        void normalisieren ();
        inline void set_x (double x){ *x_= x; }
        inline void set_y (double y){ *y_= y; }
        inline void set_z (double z){ *z_= z; }
        inline double get_x () const { return *x_; }
        inline double get_y () const { return *y_; }
        inline double get_z () const { return *z_; }
    private: 
        double* x_;
        double* y_;
        double* z_;
    };
    
    Vector3D::Vector3D (double x, double y, double z) 
    : x_(new double(x)),  y_(new double(y)), z_(new double(z))
    {}
    
    Vector3D::Vector3D ()
    : x_(new double(0)),  y_(new double(0)), z_(new double(0))
    {}
    
    Vector3D::~Vector3D ()
    {
        delete x_;
        delete y_;
        delete z_;
    }
    
    void Vector3D::operator + (Vector3D* vec)
    {
        *x_ = *x_ + *vec->x_;
        *y_ = *y_ + *vec->y_;
        *z_ = *z_ + *vec->z_;
    }
    
    void Vector3D::operator * (int x)
    {
        *x_=*x_ * x;
        *y_=*y_ * x;
        *z_=*z_ * x;
    }
    
    void Vector3D::normalisieren ()
    {
         long double laenge = sqrt (*x_ * *x_ + *y_ * *y_ + *z_ * *z_);
         if (laenge!=0) 
         {
             *x_ = *x_ / laenge;
             *y_ = *x_ / laenge;
             *z_ = *x_ / laenge;
         }
    }
    
    int main() 
    {
        Vector3D* v1 = new Vector3D(7,5,3);
        Vector3D* v2 = new Vector3D(2,-6,20);
        Vector3D* v3 = new Vector3D();
    
        *v3 + v2; 
    
        std::cout << v1->get_x() << " ";
        std::cout << v1->get_y() << " ";
        std::cout << v1->get_z() << std::endl;
    
        std::cout << v2->get_x() << " ";
        std::cout << v2->get_y() << " ";
        std::cout << v2->get_z() << std::endl;
    
        *v2 + v1;
    
        std::cout << v2->get_x() << " ";
        std::cout << v2->get_y() << " ";
        std::cout << v2->get_z() << std::endl;
    
        delete v1;  
        delete v2;
        delete v3;
        wait();
        return 0;
    }
    

    Die operator... taugen so natürlich nichts, weil sie void sind. Außerdem fehlt eine Funktion, dass man den Vektor direkt auf cout schieben kann.
    Schau Dir mal folgende Links an:
    http://www.volkard.de/vcppkold/die_klasse_rational.html
    http://www.gotw.ca/gotw/004.htm
    Da kannst Du einiges über deine aktuelle Aufgabe lernen.



  • x_ = new double (x);
    y_ = new double (y);
    z_ = new double (z);
    

    Wir sind hier nicht in Java...



  • Wenn Du Wert auf Geschwindigkeit legst (z.B. für Games, vielfache Berechnungen), solltest Du float anstelle double oder long double einsetzen und folgende Funktion für das Wurzelziehen verwenden:

    float FastSQRT( float r )
    {
        // Null und negative Zahlen abfangen
        //if( r == 0.0f ) return 0.0f;
        if( r <  0.0f ) r = -r     ;
    
        // Startwert festlegen
        float val = r;
        float halfval = 0.5f * r;
        unsigned long *ptr = (unsigned long*)&r;
        *ptr = ( 0xbe6f0000 - *ptr ) >> 1;
    
        float temp = r;
    
        // Zwei Schritte reichen
        temp *= 1.5f - temp * temp * halfval;
        temp *= 1.5f - temp * temp * halfval;
        //temp *= 1.5f - temp * temp * halfval; //höhere Präzision  
    
        // Rückgabe
        return val*temp;
    }
    

    siehe auch:
    http://ktd.club.fr/programmation/fichiers/FastMath.h



  • Wir sind hier nicht in Java...

    😃 Nein, bei C++: 😉
    http://www.cppreference.com/keywords/new.html

    pointer = new type( initializer ); //An optional initializer can be used to initialize the memory.
    

    Das wird in vielen Lehrbüchern irgendwie tot geschwiegen.



  • Ich meinte damit das man in C++ nicht alles mit new anlegt, sondern nur da wo es Sinn macht. 😉



  • Klar geht das alles ganz anders, aber es passte gerade so schön.
    Siehe vor allem hier: http://www.c-plusplus.net/forum/viewtopic-var-t-is-154875-and-highlight-is-vektor.html



  • Herzlichen Dank für die Verbessetrungen.

    Mich hat aber auch interresiert, wie ich das in eine eigene Datei packe und dafür einen Header erstelle. (Was muß alles in so einen Header rein?)

    Jetzt habt ihr geschrieben das sei "zufiel", warum?
    Wie würde man die Vectorklasse den in eine eigene Datei schreiben und wie säh der header dazu aus?



  • Dir muss man aber auch alles zeigen:

    //Header Vector3D.h
    
    #pragma once //oder normaler Header-Guard
    
    #include <cmath> //für sqrt(...), oder besser FASTSqrt(...) und float
    
    class Vector3D
    {
    public:
        Vector3D (double , double , double );
        Vector3D ();
        void operator + (Vector3D*);
        void operator * (int);
        ~Vector3D ();
        void normalisieren ();
        inline void set_x (double x){ *x_= x; }
        inline void set_y (double y){ *y_= y; }
        inline void set_z (double z){ *z_= z; }
        inline double get_x () const { return *x_; }
        inline double get_y () const { return *y_; }
        inline double get_z () const { return *z_; }
    private:
        double* x_;
        double* y_;
        double* z_;
    };
    
    //Vector3D.cpp
    
    #include Vector3D.h
    
    Vector3D::Vector3D (double x, double y, double z)
    : x_(new double(x)),  y_(new double(y)), z_(new double(z))
    {}
    
    Vector3D::Vector3D ()
    : x_(new double(0)),  y_(new double(0)), z_(new double(0))
    {}
    
    Vector3D::~Vector3D ()
    {
        delete x_;
        delete y_;
        delete z_;
    }
    
    void Vector3D::operator + (Vector3D* vec)
    {
        *x_ = *x_ + *vec->x_;
        *y_ = *y_ + *vec->y_;
        *z_ = *z_ + *vec->z_;
    }
    
    void Vector3D::operator * (int x)
    {
        *x_=*x_ * x;
        *y_=*y_ * x;
        *z_=*z_ * x;
    }
    
    void Vector3D::normalisieren ()
    {
         long double laenge = sqrt (*x_ * *x_ + *y_ * *y_ + *z_ * *z_);
         if (laenge!=0)
         {
             *x_ = *x_ / laenge;
             *y_ = *x_ / laenge;
             *z_ = *x_ / laenge;
         }
    }
    
    //main.cpp
    
    int main()
    {
    //...    
    return 0;
    }
    


  • Hi!

    void operator + (Vector3D*); 
    void operator * (int);
    

    Als kleinen Vorschlag, schreib die Operatoren so:

    Vector3D operator +( const Vector3d &v );
    Vector3D operator *( float f );
    

    Denn, bei dem + Operator wird normalerweise nicht das Objekt selbst geändert:

    Vector3D v1( 10.0f, 20.0f, 30.0f );
    Vector3D v2( 1.0f, 2.0.f, 3.0f );
    Vector3D v3 = v1 + v2; // v1+v2 per "=" an v3
    

    Sowas ginge mit deinem Code garnicht, obwohl es logisch erscheint, nicht wie folgendes:

    Vector3D v1( 10.0f, 20.0f, 30.0f );
    Vector3D v2( 1.0f, 2.0.f, 3.0f );
    v1 + v2; // v1 ist jetzt (11.0, 22.0 33.0)
    

    Das wäre eher ein Fall für den "+=" Operator:

    Vector3D &operator+=( const Vector3 &v );
    Vector3D &operator*=( float f );
    
    Vector3D v1( 10.0f, 20.0f, 30.0f );
    Vector3D v2( 1.0f, 2.0.f, 3.0f );
    v1 += v2; // v1 ist jetzt (11.0, 22.0 33.0)
    

    Und wenn wirs schonmal von Geschwindigkeit haben:

    void Vector3D::normalisieren () 
     { 
          long double laenge = sqrt (*x_ * *x_ + *y_ * *y_ + *z_ * *z_); 
          if (laenge!=0) 
          { 
              *x_ = *x_ / laenge; 
              *y_ = *x_ / laenge; 
              *z_ = *x_ / laenge; 
          } 
     }
    

    Fließkommadivisionen sind irre langsam, viel schneller sind Multiplikationen:

    void Vector3D::normalisieren () 
     { 
          // Inverse Quadratwurzel ziehen (1.0f / sqrt(xyz))
          long double invlen = invsqrt( *x_ * *x_ + *y_ * *y_ + *z_ * *z_); 
          if (laenge!=0) 
          { 
              *x_ = *x_ * invlen;
              *y_ = *x_ * invlen; 
              *z_ = *x_ * invlen; 
          } 
     }
    

    Wenn du dann noch eine schnelle Implementierung für Inverse Quadratwurzeln verwendest gewinnst du einiges an Geschwindigkeit. Aber selbst 1.0 / sqrt() würde einiges an Speed bringen.

    grüße



  • Übrigens solltest du deiner Klasse noch einen operator= und einen Kopier-Kopistruktor spendieren, sonst landest du sehr schnell im Bereich der Speicherlecks oder (noch viel schlimmer) Zugriffsfehler:

    Vector3D v1(1,1,1),v2(2,2,2),v3;
    v3 = v1+v2;
    //Speicherleck   - alte x/y/z unerreichbar
    //Zugriffsfehler - op+ hat temporäre Variablen gelöscht
    


  • Nein er sollte einfach nicht new verwenden.



  • Nein er sollte einfach nicht new verwenden.

    Korrekt, aber er wollte es aus Übungsgründen so haben.



  • Hallo, ich habe eine neue Version OHNE new IN der Vectorklasse versucht.
    Allerdings stürzt das Programm jetzt beim + operator ab.

    Es sieht jetzt so aus:

    Verctor3D.H:

    class Vector3D {
    
    private: 
        float x_; 
        float y_; 
        float z_; 
    private:inline float Vector3D::FastSQRT( float);
    
    public:		 
    	Vector3D (float , float , float ); 
        Vector3D (); 
    	bool operator == (const Vector3D); 
        Vector3D* operator + (const Vector3D); 
        void operator * (const int); 
        ~Vector3D (); 
        void normalisieren (); 
    	float laenge();
        inline void Vector3D::set_x (float x){ x_= x; } 
        inline void Vector3D::set_y (float y){ y_= y; } 
        inline void Vector3D::set_z (float z){ z_= z; } 
        inline float Vector3D::get_x () const { return x_; } 
        inline float Vector3D::get_y () const { return y_; } 
        inline float Vector3D::get_z () const { return z_; } 
    
    };
    

    Vector3D.cpp

    #include "Vector3D.h"
    Vector3D::Vector3D (float x, float y, float z) 
    : x_(x),  y_(y), z_(z) 
    {} 
    
    Vector3D::Vector3D () 
    : x_(0),  y_(0), z_(0) 
    {} 
    
    Vector3D::~Vector3D () 
    { 
    
    } 
    
    bool Vector3D::operator == (const Vector3D vec) 
    { 
    	if ((x_ == vec.x_) && (y_ == vec.y_) && (z_ == vec.z_)) {return true;}
    	return false;
    
    } 
    
    Vector3D* Vector3D::operator + (const Vector3D vec) 
    { 
    
    Vector3D *vec2 =new Vector3D(x_ + vec.x_,y_ + vec.y_,z_ + vec.z_); //Hier erfolgt der Absturz
    return vec2;
    
    } 
    
    void Vector3D::operator * (const int x) 
    { 
        x_=x_ * x; 
        y_=y_ * x; 
        z_=z_ * x; 
    } 
    
    inline float Vector3D::FastSQRT( float r ) 
    { 
        // Null und negative Zahlen abfangen 
        if( r == 0.0f ) return 0.0f; 
        if( r <  0.0f ) r = -r     ; 
    
        // Startwert festlegen 
        float val = r; 
        float halfval = 0.5f * r; 
        unsigned long *ptr = (unsigned long*)&r; 
        *ptr = ( 0xbe6f0000 - *ptr ) >> 1; 
    
        float temp = r; 
    
        // Zwei Schritte reichen 
        temp *= 1.5f - temp * temp * halfval; 
        temp *= 1.5f - temp * temp * halfval; 
        //temp *= 1.5f - temp * temp * halfval; //höhere Präzision   
    
        // Rückgabe 
        return val*temp; 
    } 
    
    void Vector3D::normalisieren () 
    { 
         float laenge = FastSQRT (x_ * x_ + y_ * y_ + z_ * z_); 
         if (laenge!=0) 
         { 
             x_ = x_ / laenge; 
             y_ = x_ / laenge; 
             z_ = x_ / laenge; 
         } 
    }
    
    float Vector3D::laenge () 
    { 
         return FastSQRT (x_ * x_ + y_ * y_ + z_ * z_); 
    
    }
    

    main:

    #include <cmath> 
    #include <iostream> 
    #include "Vector3D.h"
    
    void wait() 
    { 
        std::cin.clear(); 
        std::streambuf* pbuf = std::cin.rdbuf(); 
        std::streamsize size = pbuf->in_avail(); 
        std::cin.ignore(size); 
        std::cin.get(); 
    } 
    
    int main() 
    { 
        Vector3D* v1 ;
        Vector3D* v2 = new Vector3D(2,-6,20); 
        Vector3D* v3 = new Vector3D(); 
    
        v1 = *v1 + *v2; 
    
        delete v1;   
        delete v2; 
        delete v3; 
        wait(); 
        return 0; 
    }
    

    Das Problem ist beim erzeugen des Vector3D in der Methode für den + Operator.
    Ich bekomme die Meldung, das in fremden Speicher geschrieben wird.
    Ich wollte extra darauf achten, das das Objekt noch existiert wenn die Methode verlassen wird zur rückgabe und wollte es deswegen mit new erzeugen.
    Aber genau da stürzt es ab.
    Warum?



  • Hi!

    bool Vector3D::operator == (const Vector3D &vec) const
    {
        return ( ( x_ == vec.x_ ) && y_ == vec.y_ ) && ( z_ == vec.z_ ) );
    }
    
    Vector3D Vector3D::operator + (const Vector3D &vec) const
    { 
         return Vector3D( x_+vec.x_, y_+vec.y_, z_+vec.z_ ); 
    }
    
    Vector3D Vector3D::operator * (const int x) const
    {
        return Vector3D( x_*x, y_*x, z_*x );
    }
    
    void Vector3D::normalisieren () 
    {
         float invLen = 1.0f / FastSQRT (x_ * x_ + y_ * y_ + z_ * z_);
         if (laenge!=0)
         {
             x_ = x_ * invLen;
             y_ = y_ * invLen;
             z_ = z_ * invLen;
         }
    }
    

    inline float Vector3D::FastSQRT( float r ) hat egtl nichts in der Klasse verloren...

    float Vector3D::laenge () const
    {
         return FastSQRT (x_ * x_ + y_ * y_ + z_ * z_);
    
    }
    

Log in to reply