Fehler beim Operator überladen



  • Hallo beinander,

    ich habe mit eurer Hilfe Matrizen und Vektoren als Templateklassen erstellt und arbeite seitdem erfolgreich damit.

    Grundidee der Klassen:

    mymatrix.h

    #ifndef MYMATRIX_H
    #define MYMATRIX_H
    
    #include "myvector.h" // für Matrix-Vektor-Operationen
    
    // Abkürzungen
    #define VECTOR DoubleVector<R>
    #define MATRIX DoubleMatrix<R,C>
    
    template <int R, int C> // R Zeilen, C Spalten
    class DoubleMatrix {
    ...
    
    };
    
    #undef VECTOR
    #undef MATRIX
    #endif
    

    myvector.h

    #ifndef MYVECTOR_H
    #define MYVECTOR_H
    
    #include "mymatrix.h" // Für Matrix-Vektor-Operationen
    
    // Abkürzungen
    #define MATRIX DoubleMatrix<L,L>
    #define VECTOR DoubleVector<L>
    
    template <int L> // L Dimension
    class DoubleVector {
    ...
    };
    
    #undef VECTOR
    #undef MATRIX
    #endif
    

    Vom aufrufenden Programm wird nur einer der Header eingebunden, weil der andere ja automatisch folgt.

    Verschiedenste Methoden und Operatoren (u. a. Matrix * Vektor als Operator der Matrixklasse) funktionieren korrekt.
    Jetzt wollte ich die Funktionalität der Vektorklasse um eine Multiplikation Vektor * Matrix erweitern.

    In myvector.h:

    VECTOR& operator*( MATRIX &matrix );
    

    Dies wirft allerdings beim Kompilieren mit gcc47 (g++ -std=c++1 ...) folgende Fehlermeldungen:

    `myvector.h:37:21: error: declaration of 'operator*' as non-function

    myvector.h:37:18: error: expected ';' at end of member declaration

    myvector.h:37:33: error: expected ')' before '<' token`

    Woher kommt diese?

    Ändern der entsprechenden Zeile zu

    DoubleVector<L>& operator*( DoubleMatrix<L,L> &matrix );
    

    brachte keine Besserung.


  • Mod

    Da wird irgendetwas schief gegangen sein mit den Makros. Kein Wunder, denn Makros sind äußerst frickelig. Warum benutzt du Makros? Konkret kann man den Fehler an dem von dir gezeigten Code nicht nachvollziehen, siehe dritter Link in meiner Signatur.



  • Ich wollte gerade sagen, dass der Code so ueberhaupt keinen Sinn ergibt mit dem Template. Nach dem Kommentar von SeppJ bin ich jetzt allerdings gerade etwas verwirrt (ich nehme an, dass das ein Feature von C++ ist, das ich nicht kenne).

    Kann mir mal jemand sagen, was

    template <int R, int C> // R Zeilen, C Spalten
    

    macht?
    Warum die Anzahl Zeilen und Spalten als Template-Parameter. Das uebergibt man doch im Konstruktor? 😕


  • Mod

    icarus2 schrieb:

    Ich wollte gerade sagen, dass der Code so ueberhaupt keinen Sinn ergibt mit dem Template.

    Über Sinn und Unsinn habe ich nichts gesagt. Dazu ist zu wenig von dem Code zu sehen.

    Warum die Anzahl Zeilen und Spalten als Template-Parameter. Das uebergibt man doch im Konstruktor? 😕

    Weil sie hier wohl Compilezeitkonstanten sein sollen. War dir nicht bekannt, dass man Templates auch mit integralen Parametern statt Typnamen benutzen kann? Das kann man ziemlich oft gebrauchen. Ein typisches Beispiel ist std::array.



  • SeppJ schrieb:

    War dir nicht bekannt, dass man Templates auch mit integralen Parametern statt Typnamen benutzen kann? Das kann man ziemlich oft gebrauchen.

    Das hatte ich vorher noch nie gesehen. Wieder mal was gelernt 🙂



  • Hallo,

    ich dachte, das wäre ein einfaches, typisches Problem... aber ok, ich stelle den Code am Abend zur Ansicht, wenn ich wieder zuhause bin, wo die Dateien liegen 🙂


  • Mod

    nmn schrieb:

    Hallo,

    ich dachte, das wäre ein einfaches, typisches Problem... aber ok, ich stelle den Code am Abend zur Ansicht, wenn ich wieder zuhause bin, wo die Dateien liegen 🙂

    Es ist eben untypisch, Makros zu benutzen. Wo siehst du hier denn einen Vorteil? Du scheinst es wie einen typedef zu nutzen, jedoch mit allen Nachteilen eines Makros.



  • vor allem werden probleme kommen, wenn du diese definition ausserhalb deiner klasse nutzt.

    klar, du denkst durch das undef geht das nicht. aber das ist falsch, durch das undef trägst du das nicht raus, aber innerhalb der datei kannst dir damit schon noch sehr einfach ins knie schiessen...

    typedef und gut ist.

    @icarus2: so funktioniert auch die compilezeit evaluationen von berechnungen
    bsp:

    template<int I>
    struct factorial
    {
        enum { value = I * factorial<I-1>::value; };
    };
    
    template<0>
    struct factorial
    {
        enum { value = 1; };
    };
    
    int main()
    {
        std::cout << factorial<6435>::value << std::endl;
    
        return 0x0;
    }
    


  • SeppJ schrieb:

    Es ist eben untypisch, Makros zu benutzen. Wo siehst du hier denn einen Vorteil? Du scheinst es wie einen typedef zu nutzen, jedoch mit allen Nachteilen eines Makros.

    Hallo SeppJ,

    der Grund war einfach, dass ich das benutzt habe, um weniger tippen zu müssen. Leider habe ich wenig Erfahrung mit C/C++ generell, so dass ich nicht wusste, dass das keine so gute Idee ist.
    Auch ist mir nicht klar, wie ich die Arraygrenzen anders definieren sollte als über die Templateparameter.

    Zu meiner eigentlichen Frage:
    Ich habe jetzt die Makros gelöscht, aber das Problem bleibt erhalten.

    Der komplette Code ist zu finden auf pastebin:
    mymatrix.h
    myvector.h
    Mir ist auch klar, dass ich mangels Wissens noch einiges an ungewöhnlichem/schlechten C++-Stil drinhaben werde und sicher die Möglichkeiten nicht ausschöpfe, aber natürlich bin ich für Verbesserungsvorschläge offen.

    Minimalversion durch copy&paste ist folgendes:

    myvector.h

    #ifndef MYVECTOR_H
    #define MYVECTOR_H
    
    #include <array>
    #include "mymatrix.h"
    
    template <int L>
    class DoubleVector {
    public:
        DoubleVector()  
        : dim( L ) {}
    
        DoubleVector( double o[L] )         
        : dim( L ) {                
            for ( int i = 0; i < dim; i++ )
                v[i] = o[i];
        }
    
        DoubleVector<L>& operator=( DoubleVector< L >& other ) {
            for ( int i = 0; i < dim; i++ )
    	    v[i] = other.v[i];
    	return *this;
        }
    
        /*      DoubleVector<L>& operator*( DoubleMatrix<L,L> &mat ) {
            return m * (*this);
    	}*/
    
        std::array< double, L > v;
        const int dim;
    };
    
    #endif
    

    mymatrix.h

    #ifndef MYMATRIX_H
    #define MYMATRIX_H
    
    #include <array>
    #include "myvector.h"
    
    template <int R, int C>
    class DoubleMatrix {
        public:
    
            DoubleMatrix( double data[C][R] )
                : rows( R ), cols( C ) {
                for ( int i = 0; i < rows; i++ )
                    for ( int j = 0; j < cols; j++ )
                        m[i][j] = data[i][j];
            }
    
            DoubleVector<R>& operator*( DoubleVector<R> &v ) {
                DoubleVector<R> *tmp = new DoubleVector<R>;
                for ( int i( 0 ); i < v.dim; i++ ) {
                    double sum( 0 );
                    for ( int j = 0; j < cols; j++ )
                        sum += m[i][j] * v.v[j];
    		tmp->v[i] = sum;
                }
                return *tmp;
            }
    
        bool decomposed;
        std::array< std::array< double, C >, R > m;
        std::array< int, R > lines;
        const int cols, rows;
    };
    
    #endif
    

    test.cc

    #include "mymatrix.h"
    #include <cstdio>
    
    using namespace std;
    
    int main() {
      double vecValues[3] = {1,2,3};
      double zero[3] = {0,0,0};
      double matValues[3][3] = {{1,2,3},{4,5,6},{7,8,9}};
      DoubleVector<3> vec( vecValues ), res1, res2;
      DoubleMatrix<3,3> mat( matValues );
    
      res1 = mat * vec;
      for ( int i = 0; i < 3; i++ )
        printf( "%lf\t", res1.v[i] );
      printf( "\n" );
    
      /*
       res2 = vec * mat;
      for ( int i = 0; i < 3; i++ )
        printf( "%lf\t", res2.v[i] );
      printf( "\n" );
      */
    
      return 0;
    }
    

    Das kompiliert und läuft problemlos.
    Wenn ich jetzt die auskommentierten Teile aber auch dazunehme bekomme ich den Fehler vom Anfang:

    `%> g++ -std=c++11 test.cc -o test

    In file included from mymatrix.h:5:0,

                 from test.cc:1:
    

    myvector.h:25:33: error: declaration of 'operator*' as non-function

    myvector.h:25:30: error: expected ';' at end of member declaration

    myvector.h:25:45: error: expected ')' before '<' token

    `



  • So... ich (nmn) habe mich jetzt registriert.
    Die Variable zero ist natürlich ebenfalls überflüssig, wird auch nirgends benutzt.



  • mymatrix.h benutzt myvector.h, das mymatrix.h benutzt. Eins kommt halt zuerst, der andere Typ ist dann noch unbekannt.



  • du includest in myvector deine mymatrix rein. und umgekehrt. das nennt man zyklische abhängigkeit und das mag ein compiler nicht wirklich. und du brauchst das nichtmals, wenn du deine beiden klassen immer nur gleichzeitig inkluden lassen willst, dann include beide in eine separate header datei du dein nutzer dann includen muss.

    edit: ups verlesen, du brauchst die matrix doch im vector und umgekehrt. dann google mal nach forward declaration

    DoubleVector<L>& operator*( DoubleMatrix<L,L> &mat ) // du gibst eine referenz auf einen vektor zurück, ok
    {
        return m * (*this); // das hier ist ein temporäres objekt, es hat nichtmals einen namen
    // korrigiert mich wenn ich falsch liege, bin nicht so fit damit, aber das müsste doch ein RValue sein?!
    }
    

    auf gut deutsch, du gibst eine referenz auf ein objekt zurück, welches gar nicht mehr existiert

    (in den pastebin codes hast du es teilweise anders (schlimmer) gelöst: du holst dir manuell speicher mit new und gibst das ergebnis per zeiger zurück (wer löscht das?) ---> ganz böse



  • Das ist so eine Gewohnheit aus Java, wo ja der Garbage Collector alles einsammelt 😃

    Ich verstehe aber die Argumentation nicht, da ja das Objekt das ich als Zeiger anlege als Ergebnis zurückgegeben und weiterverwendet wird. Wenn ich einfach nur ein Objekt anlege (ohne new...) und zurückgebe, dann erhalte ich einen Segmentation Fault. Wie müsste ich es denn korrekt anlegen um es zurückgeben und weiterverwenden zu können?

    Ihr seid also der Meinung, das eigentliche Problem entsteht aufgrund zyklischer Abhängigkeit, was ich durch forward declaration lösen könnte?

    Skym0sho0:
    Zur Codefrage: hier erstellt DoubleMatrix::operator*(DoubleVector) ein Objekt, welches ich nur weiterreiche.



  • return by value ist das Zauberwort. Entferne also die Referenz(&) aus dem Rückgabetyp.

    Und warum new so schlecht ist: wer räumt denn hinterher den Speicher wieder auf?



  • Ist Return by Value nicht extrem langsa, weil es jedesmal eine kompletten Klasse übergibt statt nur die 4 byte für den Zeiger?





  • Zur Codefrage: hier erstellt DoubleMatrix::operator*(DoubleVector) ein Objekt, welches ich nur weiterreiche.

    Ja... nur dass der Rückgabewert des Funktionsaufrufes ein lvalue und kein (p)rvalue ist.
    Dadurch wird nichts kopiert sondern nur die Temporary referenziert, und dein Temporary dass du im return -Statement erzeugst, ist beim Rücksprung schon zerstört!

    NacMacNeedle schrieb:

    Das ist so eine Gewohnheit aus Java, wo ja der Garbage Collector alles einsammelt 😃

    Gewöhn dir das ganz schnell ab. Und gewöhn dir am Besten auch komplett ab, standardmäßig vom Freispeicher zu allokieren. Das ist verdammt fehleranfällig.
    Stackobjekte wo es geht!

    NacMacNeedle schrieb:

    Ich verstehe aber die Argumentation nicht, da ja das Objekt das ich als Zeiger anlege als Ergebnis zurückgegeben und weiterverwendet wird. Wenn ich einfach nur ein Objekt anlege (ohne new...) und zurückgebe, dann erhalte ich einen Segmentation Fault. Wie müsste ich es denn korrekt anlegen um es zurückgeben und weiterverwenden zu können?

    Hui. Vielleicht hier ganz auf Referenzen und Freispeicherallokierung verzichten! Das wäre das beste und sowieso logischste.

    template <int L> 
    DoubleVector< L >& DoubleVector<L>::operator*( double s ) { 
        DoubleVector<L> *res = new DoubleVector<L>; 
        for ( int i = 0; i < dim; i++ ) 
            res->set( i, v[i]*s ); 
        return *res; 
    }
    

    ➡

    template <int L> 
    DoubleVector< L > DoubleVector<L>::operator*( double s ) 
    { 
        DoubleVector<L> res; 
    
        for ( int i = 0; i < dim; i++ ) 
            res->set( i, v[i]*s ); 
    
        return res; 
    }
    

    P.S.: Wieso ist die Funktion eigentlich nicht const -qualifiziert? *this wird doch in keinster Weise verändert...



  • Ok, ich verstehe den Einwand (und danke für den Link, Nope).

    Ich weiß generell ach, wie forward declaration mit Funktionen hinhaut (es passiert ja beim definieren einer Klasse mit Membervariablen etc. nix anderes).

    Aber wie deklariere ich eine ganze Klasse im Voraus, wenn eine solche zyklische Abhängigkeit existiert?



  • NacMacNeedle schrieb:

    Ist Return by Value nicht extrem langsa, weil es jedesmal eine kompletten Klasse übergibt statt nur die 4 byte für den Zeiger?

    Hier muss so oder so ein neues Objekt erzeugt werden. Und dank RVO wird dann auch nix mehr kopiert.
    (Wird auch nochmal im von ->nope verlinkten Artikel erläutert)



  • Ah... ich habs.

    Vorausdeklaration einer Klasse funktioniert genauso wie Vorausdeklaration sonst.
    Es tut jetzt mit

    mymatrix.h

    #ifndef MYMATRIX_H
    #define MYMATRIX_H
    
    #include <array>
    #include "myvector.h"
    
    template <int R, int C>
    class DoubleMatrix {
     public:
      DoubleMatrix( double data[R][C] );
      DoubleVector<R> operator*( DoubleVector<R> );
    
      bool decomposed;
      std::array< std::array< double, C >, R > m;
      std::array< int, R > lines;
      const int cols, rows;
    };
    
    template <int R, int C>
    DoubleMatrix<R,C>::DoubleMatrix( double data[R][C] )
      : rows( R ), cols( C ) {
      for ( int i = 0; i < rows; i++ )
        for ( int j = 0; j < cols; j++ )
          m[i][j] = data[i][j];
    }
    
    template <int R, int C> 
    DoubleVector<R> DoubleMatrix<R,C>::operator*( DoubleVector<R> v ) {
      DoubleVector<R> tmp;
      for ( int i( 0 ); i < v.dim; i++ ) {
        double sum( 0 );
        for ( int j = 0; j < cols; j++ )
          sum += m[i][j] * v.v[j];
        tmp.v[i] = sum;
      }
      return tmp;
    }
    
    #endif
    

    myvector.h

    #ifndef MYVECTOR_H
    #define MYVECTOR_H
    
    #include <array>
    #include "mymatrix.h"
    
    // Forward declaration, found in mymatrix.h
    template <int R, int C>
    class DoubleMatrix;
    
    template <int L>
    class DoubleVector {
    public:
        DoubleVector()  
        : dim( L ) {}
    
        DoubleVector( double o[L] )         
        : dim( L ) {                
            for ( int i = 0; i < dim; i++ )
                v[i] = o[i];
        }
    
        DoubleVector<L> &operator=( DoubleVector< L > other ) {
            for ( int i = 0; i < dim; i++ )
    	    v[i] = other.v[i];
    	return *this;
        }
    
        DoubleVector<L> operator*( DoubleMatrix<L,L> mat ) {
            return mat * *this;
        }
    
        std::array< double, L > v;
        const int dim;
    };
    
    #endif
    

    test.cc

    #include "mymatrix.h"
    #include <cstdio>
    
    using namespace std;
    
    int main() {
      double vecValues[3] = {1,2,3};
      double zero[3] = {0,0,0};
      double matValues[3][3] = {{1,2,3},{4,5,6},{7,8,9}};
      DoubleVector<3> vec( vecValues ), res1, res2;
      DoubleMatrix<3,3> mat( matValues );
    
      res1 = mat * vec;
      for ( int i = 0; i < 3; i++ )
        printf( "%lf\t", res1.v[i] );
      printf( "\n" );
    
      res2 = vec * mat;
      for ( int i = 0; i < 3; i++ )
        printf( "%lf\t", res2.v[i] );
      printf( "\n" );
    
      return 0;
    }
    

    Danke für eure Hilfe!
    Werde den ganzen Code demnächst anpassen und dort die Referenzen und Speicheralloziierungen weglassen.


Anmelden zum Antworten