Problem mit einer Methode



  • Hey und zwar habe ich eine Methode geschrieben die eine Einheitsmatrix bilden soll. Nun habe ich aber das Problem das die Nullen siehe unten, rein zufällig waren und er in meine else Anweisung zwar gesprungen ist aber denn Wert an der Position nicht mit 0 initialisiert hat.
    Kann mir jemand erklären warum er das nicht ausführt ?

    CMatrix::CMatrix (int dim,INI_MODE mode)
    {
    
    	 m_rows=dim;
    	 m_cols=dim;
    	 m_size =m_rows*m_cols;
    	 m_pArray=new double[ m_size];
    	if(mode==INIT_ZEROS)
    	{
    	 for(int i =0;i< m_size;i++)
    	 {
    	 	m_pArray[i]=0;
    	 }
    	}
    
    	if(mode==INIT_UNIT)
    	 {
    		 int j=0;
    		 for(int i=0; i< m_size;i++)
    		 {
    			 if(i%m_cols==0)
    			 {
    
    		 	 m_pArray[i=i+j]=1;
    		 	 j++;
    			 }
    		 else
    
    			 m_pArray[i]=0;
    
    		 }
    	 }
    }
    

    Das Wäre meine Ausgabe, hätte er an der 14ten Stelle nicht "9.77807e-315" wäre es mir vermutlich gar nicht aufgefallen.

    1 0 0 0
    0 1 0 0
    0 0 1 0
    09.77807e-315 0 1



  • Korpos schrieb:

    m_pArray[i=i+j]=1;
    

    Ist dir bewusst, dass das eine Zuweisung ist, die du innerhalb der eckigen Klammern machst, und du jedesmal j Elemente überspringst, wenn dieser Code zur Ausführung kommt?
    So wie ich den Code verstehe werden alle Elemente, die unterhalb der Hauptdiagonalen liegen überhaupt nicht initialisiert. Die ganzen Nullen im unteren linken Dreieck sind also alle "Zufall".

    Gruss,
    Finnegan



  • Danke schon einmal 🙂
    Schon das Problem war nur wenn ich m_pArray[i+j]=1 geschrieben habe hat er mir nur das Erste Element mit 1 Initialisiert. Und denn Rest mit Nullen gefüllt, was ich nicht verstanden habe.

    Wenn ich mein Array vorher mit 0 Initialisiert geht alles ohne Probleme aber das muss doch auch so gehen.

    Funktionierende Code:

    if(mode==INIT_UNIT)
    	 {
    		for(int i =0;i< m_size;i++) // Erweiterung 
    		{
    			m_pArray[i]=0;
    		}
    		 int j=0;
    		 for(int i=0; i< m_size;i++)
    		 {
    			 if(i%m_cols==0)
    			 {
    		 	 m_pArray[i=i+j]=1;
    		 	 j++;
    }
    		 }
    	 }
    }
    


  • Korpos schrieb:

    Wenn ich mein Array vorher mit 0 Initialisiert geht alles ohne Probleme aber das muss doch auch so gehen.

    Funktionierende Code:

    if(mode==INIT_UNIT)
    	 {
    		for(int i =0;i< m_size;i++) // Erweiterung 
    		{
    			m_pArray[i]=0;
    		}
    		 int j=0;
    		 for(int i=0; i< m_size;i++)
    		 {
    			 if(i%m_cols==0)
    			 {
    		 	 m_pArray[i=i+j]=1;
    		 	 j++;
    }
    		 }
    	 }
    }
    

    Wenn vorher alles genullt wurde, kann man das auch etwas kompakter schreiben (Vorschlag):

    for(int i = 0; i < m_size; i += dim + 1)
        m_pArray[i] = 1;
    

    ... also i bei jedem Schleifendurchlauf um dim + 1 erhöhen.
    Um alles kompakt in einem Rutsch zu erledigen würde ich vielleicht so etwas hier machen (sorry wenn ich nicht auf deinem Ansatz aufbaue - und Achtung! nicht getestet, gut möglich, dass ich einen Denkfehler drin hab :D):

    assert(m_size > 0);
    
    m_pArray[0] = 1;
    for (int i = 1; i < m_size; i++)
    {
        for (int next = i + dim; i < next; i++) 
            m_pArray[i] = 0;
    
        m_pArray[i] = 1;
    }
    

    Idee: Array einer korrekten Einheitsmatrix beginnt mit 1 gefolgt von dim Nullen, dann wieder eine 1, dim Nullen usw.
    Mit next bezeichne ich den nächsten Index (das nächste i) an dem eine 1 stehen muss.

    Gruss,
    Finnegan



  • Ich würde -falls noch nicht vorhanden- CMatrix eher ein set(x,y) oder [][] spendieren und dann erst die Funktionen umsetzen.

    ~~
    PS.: Kann mich zwar täuschen, aber ich kann mich nicht erinnern jemals 'unit' für Einheitsmatrix gesehen zu haben.
    Gängiger ist jedenfalls 'identitiy'.~~
    Okay, Google sagt was anderes.



  • Das Thema 2D Arrays/Matrizen hatten wir in diesem Forum schon so häufig. Und immer wieder wurden mindestens 2 Dinge vorgeschlagen:

    1. keine eigene manuelle Speicherverwaltung, sondern std::vector benutzen
    2. den operator() mit zwei Parametern überladen, um den Zugriff auf die einzelnen Elemente zu erlauben

    Außerdem gehört das Erzeugen der Einheitsmatrix nicht in den Konstruktor einer Matrix, bei enstprechendem Interface kann man das über eine freie Funktion erledigen:

    #include <vector>
    
    class Matrix
    {
       std::vector<double> Elements_;
    
    public:
       Matrix( std::size_t Rows, std::size_t Cols )
       {
          ...
       }
    
       double& operator()( std::size_t Row, std::size_t Col )
       {
          ...
       }
    };
    
    Matrix create_identity_matrix( size_t Size )
    {
       Matrix m( Size, Size );
       for( size_t i = 0; i < Size; ++i ) Matrix( i,i ) = 1;
       return m;
    }
    


  • DocShoe schrieb:

    Und immer wieder wurden mindestens 2 Dinge vorgeschlagen:

    1. keine eigene manuelle Speicherverwaltung, sondern std::vector benutzen
    2. den operator() mit zwei Parametern überladen, um den Zugriff auf die einzelnen Elemente zu erlauben

    Naja, wenn du schon damit anfängst, dann ist wohl 'Matrix sollte ein template sein' wichtiger als 'operator() muss überladen werden'.

    DocShoe schrieb:

    Außerdem gehört das Erzeugen der Einheitsmatrix nicht in den Konstruktor einer Matrix, bei enstprechendem Interface kann man das über eine freie Funktion erledigen:

    Sagt wer? Oder Begründung? Oder warum macht z.B. boost das 'falsch'?



  • Jockelx schrieb:

    DocShoe schrieb:

    Und immer wieder wurden mindestens 2 Dinge vorgeschlagen:

    1. keine eigene manuelle Speicherverwaltung, sondern std::vector benutzen
    2. den operator() mit zwei Parametern überladen, um den Zugriff auf die einzelnen Elemente zu erlauben

    Naja, wenn du schon damit anfängst, dann ist wohl 'Matrix sollte ein template sein' wichtiger als 'operator() muss überladen werden'.

    Das soll ja nur eine Hilfestellung sein. Was wichtiger ist, muss der Programmierer der es verwendet wissen. Ich finde das ist Geschmacksache ... Allerdings es als Template zu designen hätte den Vorteil, dass Du einfach von double auf float umschalten könntest.

    Jockelx schrieb:

    DocShoe schrieb:

    Außerdem gehört das Erzeugen der Einheitsmatrix nicht in den Konstruktor einer Matrix, bei enstprechendem Interface kann man das über eine freie Funktion erledigen:

    Sagt wer? Oder Begründung? Oder warum macht z.B. boost das 'falsch'?

    boost baut daraus eine uninitialisierte Matrix.

    matrix (size_type size1, size_type size2) - Allocates an uninitialized matrix that holds size1 rows of size2 elements.

    --> http://www.boost.org/doc/libs/1_58_0/libs/numeric/ublas/doc/matrix.html#11Description

    boost hat extra eine Klasse für eine Einheitsmatrix --> identity_matrix<T, ALLOC>

    Daher macht boost es eher, wie es DocShoe vorgeschlagen hat 🙂



  • nebler schrieb:

    Daher macht boost es eher, wie es DocShoe vorgeschlagen hat 🙂

    Nein, überhaupt nicht.
    Es geht darum, ob man beim Erzeugen der Matrix angeben können soll, ob es eine Einheitsmatrix ist oder ob man immer erstmal eine Default-Matrix erzeugt und anschliessend zu einer Einheitsmatrix ändert.
    Wie man das jetzt macht, ob über namend-ctor, Parameter oder eben Ableitung ist doch egal.
    Und bei boost kann man offenbar beim Erzeugen angeben, was man haben will.



  • Jockelx schrieb:

    Es geht darum, ob man beim Erzeugen der Matrix angeben können soll, ob es eine Einheitsmatrix ist oder ob man immer erstmal eine Default-Matrix erzeugt und anschliessend zu einer Einheitsmatrix ändert.

    Meine Meinung ist, dass anschließend es zu einer Einheitsmatrix zu ändern ein unglücklicher weg ist. Wozu hat man den sonst die Möglichkeiten von Polymorphie, namend-ctor ... wenn diese nicht genutzt werden. Und abgesehen davon, ist die erste Initialisierung dann für die Katze.

    Jockelx schrieb:

    Wie man das jetzt macht, ob über namend-ctor, Parameter oder eben Ableitung ist doch egal.

    Klar das Resultat ist das Gleiche. Ich finde ein If-Then-Else Code in der Initialisierung nicht für einen Dritten-(gut)-Überschaubar. Daher finde ich named-ctor / Ableitungen deutlich übersichtlicher.

    offtopic:

    Jockelx schrieb:

    Und bei boost kann man offenbar beim Erzeugen angeben, was man haben will.

    Vorweg: Ich kenne boost nicht. Kannst Du mir bitte interessehalber mal kurz schreiben welcher ctor das ist? Ich habs in der API nicht gefunden. (Falsche API angeschaut? Oder den Wald vor lauter Bäume nicht gesehen? http://www.boost.org/doc/libs/1_58_0/libs/numeric/ublas/doc/matrix.html#11Description)

    Vielen Dank & einen schönen Abend euch allen 🙂



  • Es ging nur darum, dass ich die Aussage 'sowas gehört nicht in den Ctor, sondern in eine freie Fkt' so pauschal für falsch halte. Um nix anderes.

    Bzgl. boost hast du doch die Klasse selber angegeben!?



  • Das Erzeugen der Einheitsmatrix gehört für mich deshalb nicht in den Konstruktor, weil es eine zusätzliche Fallunterscheidung und einen Parameter im Konstruktor notwendig macht. Klar, man kann dafür einen Default Wert vorgeben, aber im Prinzip sollte jeder Funktion genau einen Zweck erfüllen. Und wenn der Konstruktor "entscheiden" muss, was er erledigen soll verletzt er diese Regel.
    Außerdem zieht diese Entscheidung andere Konsequenzen für das Klassendesign nach sich, dann müsste man für die inverse und transponierte Matrix ähnliche Wege gehen. Das wird in meinen Augen dann schnell unschön, wenn man immer einen zusätzlichen Parameter braucht, um bestimmte Aufgaben zu erfüllen.

    Mal zum Vergleich:

    Matrix m1 = create_identiy_matrix( 5 );
    Matrix t2 = transpone_matrix( m );
    Matrix i3 = invert_matrix( m );
    
    Matrix m2( 5, IDENTITY );
    Matrix t2( m2, TRANSPONE );
    Matrix i2( m2, INVERSE );
    

    Da finde ich Variante 1 deutlich lesbarer.



  • 🙄
    Ich bin blind...
    Ich dachte, du hättest sowas vorgeschlagen:

    Matrix m;
    createIdentity(m);
    

    Meine Kommentare passen dann auch gar nicht zu deinem Vorschlag.



  • Gut, das ging aus meinem Post auch nicht eindeutig hervor.
    Edit: Doch 😉


Anmelden zum Antworten