Konstante Variablen werden verändert



  • Hi,
    Ich schreibe gerade eine mathematische Klasse für einen Vektor.
    Diese Klasse nutzte VORHER ein Template der Art

    template <class typeOfVariable, unsigned int numElements>
    

    um statisch Speicherplatz für jedes Element zu vergeben. Alles funktionierte wunderbar.

    Aus verschiedenen Gründen musste ich die Klasse derart umschreiben, dass sie nun Ihren Speicher dynamisch reserviert:

    template<class typeOfVariable>
    class Vector
    {
        private:
        typeOfVariable* vectorElement;
        unsigned int    numElements;
    

    Zum Ansprechen von Elementen wird der Operator [] überladen:

    template<class typeOfVariable>
    typeOfVariable& Vector<typeOfVariable>::operator[] (const unsigned int index) const
    {
        //Out of range!
        assert(index < numElements);
        return vectorElement[index];
    }
    

    Ebenfalls überladen wird der Operator = :

    template<class typeOfVariable>
    Vector<typeOfVariable> Vector<typeOfVariable>::operator=  (const Vector<typeOfVariable> v)
    {
        //Both vectors have to be of equal dimension
        if (numElements != v.getDimension())
        {
            numElements = v.getDimension();
            delete[] vectorElement;
            vectorElement = new typeOfVariable[numElements];
        }
        //Assign values to this vector
        for (unsigned int i=0; i<numElements; i++)
            vectorElement[i] = v[i];
    
        return *this;
    }
    

    Das Zuweisen von Werten (doubles) funktioniert prima. Hier zum Beispiel ein Vektor mit drei Elementen und den Werten 2, 2, 8 sowie 1, 2, 3:

    Vector<double> vec(3, 2.0, 2.0, 8.0);
    Vector<double> vec2(3, 1.0, 2.0, 3.0);
    

    Sobald ich aber schreibe...

    vec = vec2;
    

    ...erhalte ich seltsame Werte, die danach riechen, dass irgendwo in fremde Speicherbereiche hineingelesen / geschrieben wird:

    vec: 1.35943e-305 / 2 / 3
    vec2: 0 / 0 / 0

    Woran kann das liegen? Warum wird auch vec2 verändert? Die Variable ist doch als const deklariert worden.

    Danke für eure Hilfe!



  • Wie sieht dein Copy-Konstruktor aus?


  • Mod

    Hmm, eine Klasse namens Vector die sich selbst dynamisch Speicher reserviert...

    War da nicht irgendwas? Hatte glaube ich irgendwas mit der Standardbibliothek zu tun...



  • Mercator schrieb:

    Sobald ich aber schreibe...

    vec = vec2;
    

    ...erhalte ich seltsame Werte,

    Dein Code sieht eigentlich richtig aus.
    Sicher, daß er bei

    vec = vec2;
    

    passiert und nicht bei

    vec = vec;
    

    ?
    Gegen Selbstzuweisung ist er nämlich nicht sicher. Dein Code würde richtig geil abkeckern, fast immer laufen, sagen wir mal 20000 zu 1 bringen, 20000 Läufe klappen und einer nicht. Also gerade der Lauf, wo der Kunde das erste mal damit im Fernsehen zu sehen ist. Oder mit netter Debug-Bibliothek auch öfter oder jesesmal. Das Problem hier wäre, daß Du vec.zeug erst deleten würdest, um es dann nach dem neuen vec.zeug zu kopieren. Geht natürlich nicht, weil es schon deleted wurde.
    Früher verhinderte man das mit einem if(&v==this) return; als erste Zeile.
    Jetzt nimmt man das Copy&Swap-Idiom, weil es darüberhinaus noch exceptionsicher ist. Und eigentlich auch einfacher irgendwie.

    Das ist aber kein Fehler, der oft sichtbar wird, weil eine Selbstzuweisung im Anwendungscode sehr selten ist. Eigentlich nur, wenn der Anwendungscodierer umständlich ist. Oder ganz selten manchmal, wenn... (geschätzt, bei mir zweimal in drei Jahren) (strukturierte Programmierer ohne break, goto und continue vermutlich auch häufiger, bis zu einmal im Monat, frei grob geschätzt).



  • Hi,
    Danke an hmpf und volkard.

    Ich hatte keinen Copy-Konstruktor. Sobald ich ihn implementiert hatte, funktionierte bereits das Beispiel.

    So sieht er aus:

    template<class typeOfVariable>
    Vector<typeOfVariable>::Vector(const Vector<typeOfVariable>& v)
    {
          numElements   = v.getDimension();
          vectorElement = new typeOfVariable[numElements];
    
          for(unsigned int i=0;i<numElements;i++)
             vectorElement[i]=v[i];
    };
    

    Außerdem verwende ich nun das Copy&Swap-Idiom:

    template<class typeOfVariable>
    Vector<typeOfVariable> Vector<typeOfVariable>::operator=  (const Vector<typeOfVariable> v)
    {
        Vector<typeOfVariable> temp(v); // Copy constructor
        temp.swap(*this);
        return *this;
    }
    
    template<class typeOfVariable>
    void Vector<typeOfVariable>::swap(Vector<typeOfVariable>& v)
    {
          std::swap(numElements, v.numElements);
          std::swap(vectorElement, v.vectorElement);
    };
    

    Mein Code funktioniert jetzt. Vielen Dank!



  • Das hättest Du auch einfacher haben können:

    template<class T>
    class vec
    {
      std::vector<T> coeffs_;
    public:
      u.s.w.
    };
    

    (ohne Zuweisungsoperator, Kopierkonstruktor und Destruktor selbst definieren zu müssen)

    Wie hast Du denn Konstruktor gebaut, der die Koeffizienten entgegen nimmt? Ich hoffe, der sieht nicht so aus:

    vec::vec(int dim,...);
    

    Da geht nämlich leider die Typinformation verloren (so dass der Compiler das nicht prüfen kann). Du erwartest sicherlich die Typen T. Aufrufen kannst du den aber leider mit beliebigen Argumenten.



  • Dass du da Probleme bekommst, liegt wohl daran, dass du dem operator= einen Value mitgibst. So wie beim Copy-CTor sollte das eine const-Referenz sein (das const steht ja schon da, nur das "&" fehlt). Dadurch wird vor dem Zuweisen per operator= erst eine Kopie gezogen - und HIER geht es dann schief.



  • Referend Opa schrieb:

    So wie beim Copy-CTor sollte das eine const-Referenz sein

    Warum? Manchmal finde ich so'was wie

    Typ& Typ::operator=(Typ lokale_kopie)
    {
      this->swap(lokale_kopie);
      return *this;
    }
    

    total praktisch. Das könnte man hier auch machen. Aber, wie gesagt, noch besser finde ich es, wenn ich die speziellen Funktionen gar nicht selbst schreiben muss. Das ist dann weniger fehleranfällig und kürzer.



  • aber dadurch kam sicher sein Problem zustande, da er für die lokale Kopie keinen Copy-CTor hatte.

    greetz KN4CK3R



  • Ja sicher. Ich konnte das nur so als generelle Aussage nicht stehen lassen. 😃



  • krümelkacker schrieb:

    Ja sicher. Ich konnte das nur so als generelle Aussage nicht stehen lassen. 😃

    War doch gar nicht "generell". Ich fang an mit "dass du da", was sich auf seinen Kontext bezieht. Und der letzte Satz sagt auch eindeutig, dass so erst noch die Kopie gezogen wird, was bei ihm das Problem auslöst.
    Eine generell fordernde Aussage wäre "op= muss immer eine const-Referenz übergeben bekommen", und das lese ich aus meinem obigen Beitrag nicht raus.

    Seine aktuelle Lösung verwendet zwar Copy&Swap, aber es werden 2 Kopien gezogen (wäre vllt. besser gewesen ihn hierauf hinzuweisen, als auf irgend welchen Formulierungen und Generalisierungen rumzureiten)! Einmal in den const-Parameter der Funktion, und dann nochmal eine in der Funktion. Wenn du deinen op= schon per Copy&Swap implementierst, dann entweder wieder per constref übergeben und Kopie in der Funktion oder kein const-Value als Parameter sondern direkt so wie es Krümelkacker gezeigt hat. Ersteres hat den Vorteil (wenn man so will), dass die Implementierung nicht schon durch die Signatur ersichtlich ist.



  • krümelkacker schrieb:

    Das hättest Du auch einfacher haben können:

    template<class T>
    class vec
    {
      std::vector<T> coeffs_;
    public:
      u.s.w.
    };
    

    (ohne Zuweisungsoperator, Kopierkonstruktor und Destruktor selbst definieren zu müssen)

    Ja, ok. Besonders jetzt, wo der Vektor dynamisch geworden ist, ist es wirklich kürzer. Hab es geändert.
    Die Thematik mit dem Copy-Konstruktor kannte ich aber noch nicht. Also gut, dass das Problem mal auftauchte 😉

    krümelkacker schrieb:

    Wie hast Du denn Konstruktor gebaut, der die Koeffizienten entgegen nimmt? Ich hoffe, der sieht nicht so aus:

    vec::vec(int dim,...);
    

    Da geht nämlich leider die Typinformation verloren (so dass der Compiler das nicht prüfen kann). Du erwartest sicherlich die Typen T. Aufrufen kannst du den aber leider mit beliebigen Argumenten.

    Hatte schon überlegt, ob ich ihn rausnehme oder dennoch verwende. Man kann schließlich auch schlecht prüfen, ob die richtige Anzahl an Argumenten angegeben wurde. Sind es zu wenig, liest er in fremde Speicherbereiche hinein.

    Eine Zeile zum initialisieren von einem Vektor ist aber einfach praktisch.

    Als nächstes wollte ich eine Matrix implementieren, die aus Spaltenvektoren besteht. Jetzt könnte ich doch wieder den std::vector nehmen und meine mathematischen Vektoren darin speichern, oder?


  • Mod

    edit: Hat sich inzwischen erledigt.


Log in to reply