Probleme mit dem Destructor



  • 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_);
    
    }
    


  • @ MisterX

    Deine Idee mit dem new ist schon ganz richtig.
    Allerdings stimmt etwas in dem Aufruf nicht.

    v1 = *v1 + *v2;
    

    Hier benutzt du *v1 (auf der rechten Seite), obwohl du v1 noch nichts zugewiesen hast. Das ist ein Zeiger, der irgendwo ins Kraut zeigt.

    Dies müsste bei deinem code funktioniren:

    v1 = *v2 + *v3;
    

Anmelden zum Antworten