Aufgabe mit dynamischen Matrizen -> Absturz/Unvorhersebares Verhalten

[BTK]

Hallo!
Zunächst einmal bearbeite ich die selbe Aufgabe wie dieser Herr hier:
https://www.c-plusplus.net/forum/330333 .

Nun bin ich die ganze Sache allerdings etwas anders angegangen und bin mit dem "Ergebnis" meiner Schreiberei etwas überfordert. Der Compiler kompiliert fehlerfrei, das Ausführen meines Programms führt nämlich, obwohl ich den Quellcode nicht veränder, jedes Mal zu anderen Ergebnissen. Hier meine Dateien:

matrix.h

#ifndef matrix_h
#define matrix_h
#define INDEX(i,j)((i)*n+(j))

class matrix{
public:
	matrix (int m,int n);
	~matrix();

	double& operator() (int i , int j) {return a[INDEX(i,j)];};  // Operatorüberladung

	void output(matrix);
	void add(matrix, matrix, matrix);
	void mmult(matrix, matrix,matrix);
	void smult(double , matrix , matrix );

private:
	double *a;
	int m,n;	

	};

#endif

matrix.cpp

#include "matrix.h"
#include <iostream>
using namespace std;

matrix::matrix(int m,int n)
  : m(m),n(n),a(new double[n*m]) {};   							

matrix::~matrix(){delete [] a;}	      					  	   // Destruktor gibt Speicher wieder frei

void matrix::output(matrix a)
{
cout << "Ausgabe der Matrix:" << endl;

for (int i = 0; i<m; i++)
    for (int j = 0; j<n; j++){
		cout << a(i,j) << "\t";
		if(j+1 == n) {cout<<"\n"<<endl;};}
;}

void matrix :: add (matrix a, matrix b, matrix c){

for (int i = 0; i<=m; i++)
	for (int j = 0; j<=n; j++)
		c (i,j) = a(i,j) + b(i,j)
;}

void matrix :: smult(double s, matrix a, matrix c){
for (int i = 0; i<=m; i++)
	for (int j = 0; j<=n; j++)
		c (i,j) = a(i,j) * s
;}

void matrix :: mmult(matrix a, matrix b, matrix c){

for(int i=0;i<m;++i)
   for(int j=0;j<b.n;++j)
	   for(int k=0;k<=n;++k)
			c(i,j) = c(i,j) + a(i,k)*b(k,j)
;}

main.cpp

#include <iostream>                                     
#include <windows.h>
#include "matrix.h"
using namespace std;

int main(){                                             

	/*BLOCK 1
	matrix a(2,3);
	matrix b(2,3);
	matrix c(2,3);

	a(0,0)= 1; a(0,1)= 2; a(0,2)= 3;					//Init der Matrizen
	a(1,0)= 4; a(1,1)= 5; a(1,2)= 6;
	b(0,0)= 7; b(0,1)= 8; b(0,2)= 9;
	b(1,0)= 10; b(1,1)= 11; b(1,2)= 12;

	c.add(a,b,c);
	c.output(c);										//Addition 
	BlOCK 1 ENDE */

	/*BLOCK 2
	matrix d(2,2);
	matrix e(2,2);

	d(0,0)= 1; d(0,1)= 2; 								//Init der Matrizen
	d(1,0)= 3; d(1,1)= 4;
	double s = 5.1;

	e.smult(s,d,e);
	e.output(e);										//Smult durchgeführt
	BLOCK 2 ENDE */

	/*BLOCK 3 ENDE 
	matrix f(2,3);
	matrix g(3,2);
	matrix h(2,2);

	f(0,0)= 1; f(0,1)= 2; f(0,2)= 3;					//Init der Matrizen
	f(1,0)= 4; f(1,1)= 5; f(1,2)= 6;
	g(0,0)=6; g(0,1)=-1;
	g(1,0)=3; g(1,1)=2;
	g(2,0)=0; g(2,1)=-3;
    h(0,0)=0;h(0,1)=0;
	h(1,0)=0;h(1,1)=0;

	h.mmult(f,g,h);
	h.output(h); 	 						    	   //Mmult durchgeführt
	BLOCK 3 ENDE */

	return 0;
	}

Nun folgende Problematiken.
Jeder der sogenannten Blöcke (falls er nicht auskommentiert ist in der main.cpp) führen jeweils für sich zum richtigen Ergebnis der mathematischen Operation.
Block 1 und Block 2 kombiniert führen auch noch zum richtigen Ergebnis( Also beide Blöcke nicht auskommentiert) rufen aber u.U einen Programmabsturz nach richtiger Berechnung auf.

Schaltet man noch Block 3 dazu liefern Block 1 und 2 richtige Ergebnisse Block 3 aber nicht, Block 3 für sich geschaltet tut dies allerdings. Ab und zu auch hier Programmabstürze.

Block 3 führt aber in Kombination mit Block 2 zum richtigen Ergebnis, aber nicht mit Block 1.

Zusammenfassend: Jeder Block für sich das richtige Ergebnis. In Kombination mit anderen Blöcken kann Block 3 falsche Ergebnisse liefern.

Als zusätzliches Kuriosum: Der Privatemember A der ein Doppelpointer ist, ist eigentlich ein Relikt aus einem anderen Ansatz. Meines Wissens nach benutze ich den zu keinem Zeitpunkt mehr, nachdem ich ihm mit dem Konstruktor einen Wert zugewiesen habe. Kommentiere ich ihn aber aus (in der Klasse und im Konstruktor) macht mein Programm nur noch Quatsch.

Ich stehe aktuell noch ziemlich auf dem Schlauch wenns um dynamische Matrizen geht, das sieht man wahrscheinlich schon an der Klassenhandhabung, aber ich bin für jede Hilfe dankbar die mir hilft den Dämon aus meinem Sourcecode zu treiben.

Vielen lieben Dank

[Nathan]

Nun, du hast die Regel der großen Drei nicht beachtet.
Blöder Fix: Schreib einen Kopierkonstruktor und Assignmentoperator.
Intelligenter Fix: Lass die Spielichen mit new und hol dir std::vector, dann hättest du den Fehler nicht gemacht und genauso wenig das Speicherleck des unbenutzten A Pointers und das falsche delete im Destruktor.

[SeppJ]

Wenn du schon manuell mit new hantierst (wieso nutzt du nicht vector?) dann mach es richtig. Da ist mindestens die Regel der großen Drei verletzt. Und noch so einiges mehr, was aber bei solch dicken Fehlern wie dem vorherigem erst einmal egal ist. Deine beschriebenen Symptome passen auch genau zu dieser Art von Fehler. Kurz: Mit vector wär das nicht passiert.

Wozu ist überhaupt matrix::A da?

Du musst nicht nach jedem } ein ; setzen. Eigentlich ist das sogar eher die Ausnahme.

Dein überladener Operator [] macht vermutlich nicht das, was du denkst. Ist hier der Sinn von matrix::A?

Benutze einen ordentlichen Einrückungsstil, dann kann man den Code auch lesen.

Dein Matrix-Header bindet iostream ein, ohne es zu nutzen.

Dein Matrix-Header erzeugt ein Makro mit einem ganz normalen, klein geschriebenem, oft vorkommendem Bezeichner. Und nutzt es dann genau einmal.

Dein Hauptprogramm bindet windows.h ein, damit auch bloß nicht jeder deinen Quellcode einfach kopieren und testen kann, ohne vorher diese Zeile löschen zu müssen.

[BTK]

@Nathan Ok jetzt musste ich erstmal googlen was es mit dieser Dreierregel auf sich hat. Danke schonmal dafür, auch wenn ich noch nicht zu 100% alles verstanden hab.
Vorallem das mit dem Zuweisungsoperator muss ich nochmal durchsteigen.
Der Destruktor müsste nun stimmen oder?

Hab den Code auch stilistisch angepasst und alles überflüssige entfernt, Danke SeppJ du hast natürlich vollkommen recht x(
Ich muss mehr oder weniger bei new bleiben, weil dass zum Teil des Stoffes gehört den ich damit vertiefen soll. Ich kann nicht einfach std templates verwenden.

Kann mir nochmal jemand ganz kurz in ein zwei Sätzen für Dumme sagen wo das Problem liegt? Also warum genau sobald ich mehr als einen Block kompiliere nichts mehr so tut wie es soll? Was das Speicherleck hervorruft?
Die Sache ist halt das ich ja jede benutzte Matrix einzeln deklariere und ich daher dachte das ich an diesem Punkt von Fehlern frei bin.

[SeppJ]

Das Verständnis, was da schief läuft, sollte eigentlich mit dem Verständnis kommen, warum man hier die Dreierregel braucht. Du machst hier jede Menge Kopien deiner Klasse, die einfach nur (dank fehlendem Kopierkonstruktor) die Pointer in deiner Klasse kopieren. Also bloß den Zahlenwert der Pointer. Es wird nicht das kopiert, worauf sie zeigen. Und später werden diese kopierten Instanzen zerstört. Sie rufen den von dir definierten Destruktor auf. Der dann delete auf die kopierten Pointer aufruft. Und dann arbeitest du mit den anderen Instanzen der Klasse weiter, die nun immer noch Kopien der gleichen Pointer haben, die nun auf deletete Speicherbereiche zeigen. Und dann passieren folglich komische Sachen.

Noch zwei Bemerkungen:
Es wäre schön, wenn du deinen Eingangsbeitrag nicht so stark verändern würdest, damit man den Thread noch halbwegs sinnvoll lesen kann. Die Formatierung anpassen ist ok, aber die Fehler entfernen, die später diskutiert werden, ist sehr verwirrend, da nicht klar ist, an welcher Stelle des Threadsverlaufs über welche Version des Codes geredet wird.

Ich kann nicht einfach std templates verwenden.

Und wie wäre es, diese nach zu programmieren? Du hast ja schon fast einen std::vector programmiert (wenn auch bisher noch falsch, wegen der Nichtbeachtung der Dreierregel). Das kann man einmal richtig machen und dann immer wieder benutzen. Das macht auch viel mehr Sinn für den Aufbau der Klassen. Die Verwaltung von Speicherressourcen ist keine Aufgabe für eine mathematische Matrix.

[Th69]

Hallo BTK,

desweiteren hast du auch noch einen logischen Fehler bei der Verwendung der Funktionsparameter. Kennst du schon Referenzen?

Deine Funktion 'add' (sowie entsprechend die anderen auch) sollte besser so deklariert werden:

void matrix::add(const matrix &a, const matrix &b, matrix &c)

Bei den ersten beiden Parametern ersparst du dir eine Kopie (nichtsdestotrotz sollte die Dreierregel ausprogrammiert werden) und beim letzten Parameter wird durch die Referenz überhaupt erst möglich, daß die aufrufende Funktion die Änderungen mitbekommt (ansonsten hast du bisher einfach nur reine lokale Kopien der übergebenen Funktionsargumente erzeugt).

PS: Und das Makro ist wirklich "pöse"!

[why]

Dein überladener Operator [] macht vermutlich nicht das, was du denkst.

Warum? Sieht doch nicht so schlecht aus die Definition?

[Th69]

Es wurde ja auch nicht [] überladen, sondern ()

[BTK]

Th69 schrieb:

Es wurde ja auch nicht [] überladen, sondern ()

Das Sprach SeppJ grad an bezüglich nachträglichem Entfernen von Fehlern und daraaus folgenden Missverständnissen. Hatte auch [] überladen obwohl ich ihn dann nicht mehr beutzt hab. Hab ihn deshalb entfernt.

[why]

Hab sowas ähnliches vor, also so wie das Überladen mit () jetzt ist, würde das schon passen oder?

[volkard]

why schrieb:

Hab sowas ähnliches vor, also so wie das Überladen mit () jetzt ist, würde das schon passen oder?

Nö.
() überladen, wenns keine Matrix, sondern eine Funktion ist.
Wenn die Matrix einen Zugriffsoperator kriegt, dann [][].
Ansonsten at(,).