Gründe für malloc NULL zu returnen

[DragonRaider]

Hi Leute,
einen schönen guten Tag erstmal ich schreibe gerade eine Matrizen-Klasse(ich weiß, C++), jedoch macht mir malloc() Probleme. Undzwar(wie schreibt man das? ^^) allociere ich in einer for()-Schleife mehrmals ein double Array. Das ist auch so weit gut und schön, funktioniert auch. Wenn der Schleifen-Durchlauf nun zuende ist(->auch der Destruktor der Klasse in der es erstellt wird wird aufgerufen) und das ganze von neuem beginnt, gibt malloc() aber plötzlich beim Allocieren des Speicherns für eine temporäre Matrix NULL zrück. Und das obewohl ich über 12 Gigabyte RAM frei habe und das Programm auch nur 45 MB verbraucht so weit ich das nachvollziehen kann wird mein Speicher auch immer schön über den Destruktor gefreet, daran dürfte es also auch nicht liegen fällt euch vielleicht aus dem Stegreif ein Grund dafür ein? Wenn nicht arbeite ich halt mit new und delete :p aber der Grund würde mich auch mal interessieren
MfG
DragonRaider

[Singender Holzkübel]

Zeig mal Code.

Wieso verwendest du überhaupt malloc/free in C++?

[speicheritis]

DragonRaider schrieb:

Hi Leute,
einen schönen guten Tag erstmal ich schreibe gerade eine Matrizen-Klasse(ich weiß, C++), jedoch macht mir malloc() Probleme.

->Using C++
->Using malloc

[] Du hast den Sinn von new und delete verstanden.

DragonRaider schrieb:

Undzwar(wie schreibt man das? ^^) allociere ich in einer for()-Schleife mehrmals ein double Array. Das ist auch so weit gut und schön, funktionier auch.

"alloziieren" ist der Infinitiv.
Und hoffentlich machst du auch schön free/delete , sonst leakst du Speicher.

DragonRaider schrieb:

Wenn der Schleifen-Durchlauf nun zuende ist(->auch der Destruktor der Klasse in der es erstellt wird wird aufgerufen) und das ganze von neuem beginnt, gibt malloc() aber plötzlich beim Allocieren des Speicherns für eine temporäre Matrix NULL zrück. Und das obewohl ich über 12 Gigabyte RAM frei habe und das Programm auch nur 45 MB verbraucht

malloc kann fehlschlagen, wenn du ordentlich viel Speicher oder 0 Bytes Speicher anforderst. Auch, wenn der Speicher arg fragmentiert ist (Billigbetriebssysteme wie Windows (;)) sind davon häufiger betroffen) oder du zwar Speicher reserviert hast, dieser aber nicht zu physikalischem Speicher gemappt wird (über MMU), kann malloc fehlschlagen. Mal vor dem Call ausgeben, was du für Parameter übergibst, ist die Basis jedes Debuggings.

new und delete werden da vermutlich die gleichen Probleme wie malloc und free aufweisen.

Da du keinen Quellcode angegeben hast, ist das alles Spekulation.
Aber für den Fall, dass du das hier machst:

while(solange_icke_lustisch_bin)
{
        mein_typ*mein_var=malloc(viele_viele_bunte_bytes);
        /*Mach voll geheime Sachen*/
        mein_var.~mein_typ();
}

Dann lass dir gesagt sein, dass der explizite Aufruf des Destruktors NICHT den Speicher freigibt, den du mit malloc geholt hast. Der Destruktor kümmert sich nur um den Speicher, der im Klassenobjekt alloziiert wird - und der Standarddestruktor auch nur dann, wenn die Objekte auf dem Stack liegen. Für alles andere musst du noch mal free aufrufen.

[DragonRaider]

Keine Ahnung ich nehme jetzt wohl einfach new und delete und fertig ist es. Ihr müsst euch echt nicht meine ganze Klasse antun xD trotzdem danke

[DragonRaider]

Und nein so mache ich es nicht ^^ Jetzt einfach mal meine auf Konstruktor und Destruktor minimierte Klasse und die sehr stark minimierten Aufrufe:

#ifndef MATRIX_LIBRARY_H
#define MATRIX_LIBRARY_H

#include <opencv2/core/core.hpp>
#include <opencv2/highgui/highgui.hpp>
#include <opencv2/imgproc/imgproc.hpp>

class Matrix
{
public:
    Matrix(unsigned const int m, unsigned const int n);                //m = rows, n = columns
    ~Matrix();

    unsigned const int getN();
    unsigned const int getM();

    Matrix operator*(Matrix b);
    double *operator[](unsigned const int i);
    Matrix& operator=(Matrix other);

    void print();

private:
    unsigned int _n, _m;
    double *_data;
};

#endif

#include "mat.h"
#include <stdexcept>
#include <cstdlib>
#include <cstring>
#include <iostream>

Matrix::Matrix(unsigned const int m, unsigned const int n) : _n(n), _m(m)
{
    _data = (double*)malloc(sizeof(double) * _m * _n);
    if(_data == NULL)
    {
        std::cerr << "_data == NULL" << std::endl;
        throw std::bad_alloc();
    }
    memset(_data, 0, sizeof(double) * _m * _n);
}

Matrix::~Matrix()
{
    free(_data);
}

(Alles mögliche includieren)
while(true)
{
    Matrix A(1, 5);
}

Ich kann auch gerne den gesamten Quellcode senden, aber ich will es jetzt mal nicht übertreiben... Ihr habt ja auch besseres zu tun
MfG
DragonRaider

Edit: Und ja ich weiß, es gäbe da auch noch cv::Mat

[oenone]

Ich denke nicht, dass in den geposteten Schnippseln der Grund für das NULL sichtbar ist.

[Nathan]

DragonRaider schrieb:

Und nein so mache ich es nicht ^^

Und wieso nutzt du nicht new/delete?
Sie haben nur Vorteile gegenüber malloc. Sie beachten Konstruktoren und Destruktoren, es gibt die Möglichkeit callbacks im Fehlerfall zu installieren und du kannst sie überschreiben für effizientere Allokation o. ä.
Sie werden zwar höchstwahrscheinlich nicht dein Problem lösen (du kriegst dann eine Exception anstatt NULL), sind aber ansonsten besser.

Der beste Weg ist aber, rohe dynamische Allokation ganz zu vermeiden. In diesem Fall bietet sich std::vector an, dann hast du auch keinen Stress wegen dem fehlenden Kopierkonstruktor.

[Furble Wurble]

Wie oenone sehe ich jetzt in diesem "Beispiel" auch nicht den Grund für Deine Probleme.

Allerdings frage ich mich, warum Du so einen eigensinnigen Stil entwickeln willst. malloc() / free() , die ganzen const in der Schnittstelle...

Das mit dem memset() macht übrigens nur, das was Du willst, wenn die Repräsentation von 0.0 als double auch nur aus lauter 0 Bytes besteht (Was aber zu Deinem Glück bei IEEE754 der Fall ist - jedenfalls +0.0)

Wo ist denn da eigentlich der Kopierkonstruktor? Und wie hast Du den Zuweisungsoperator implementiert?

[DragonRaider]

Hallo,
OK, danke. Dann werde ich mal meinen Sourcecode senden... Es wäre sehr nett, würde sich jemand das ganze mal ansehen. Danke schon mal im Voraus

mat.h:

#ifndef MATRIX_LIBRARY_H
#define MATRIX_LIBRARY_H

#include <opencv2/core/core.hpp>                //Das kann eigentlich auch raus, ist noch Altlast
#include <opencv2/highgui/highgui.hpp>
#include <opencv2/imgproc/imgproc.hpp>

class Matrix
{
public:
    Matrix(unsigned const int m, unsigned const int n);                //m = rows, n = columns
    ~Matrix();

    unsigned const int getN();
    unsigned const int getM();

    Matrix operator*(Matrix b);
    double *operator[](unsigned const int i);
    Matrix& operator=(Matrix other);

    void print();

private:
    unsigned int _n, _m;
    double *_data;
};

#endif

mat.cpp:

#include "mat.h"
#include <stdexcept>
#include <cstdlib>
#include <cstring>
#include <iostream>

Matrix::Matrix(unsigned const int m, unsigned const int n) : _n(n), _m(m)
{
    std::cout << "    Entered Matrix::Matrix(unsigned const int m, unsigned const int n)" << std::endl;
    _data = new double[_m * _n];
    std::cout << "    Allocated double* _data" << std::endl;
    memset(_data, 0, sizeof(double) * _m * _n);
    std::cout << "    Set values to 0" << std::endl;
}

Matrix::~Matrix()
{
    //free(_data);
    delete [] _data;
}

unsigned const int Matrix::getN()
{
    return _n;
}

unsigned const int Matrix::getM()
{
    return _m;
}

Matrix& Matrix::operator=(Matrix other)
{
    if(this != &other)
    {
        double *tmp_data = new double[other.getM() * other.getN()];
        std::cout << "    Allocated double* tmp_data" << std::endl;
        if(tmp_data == NULL)
        {
            std::cerr << "tmp_data == NULL" << std::endl;
            throw std::bad_alloc();
        }

        for(unsigned int i = 0; i < other.getM(); ++i)
        {
            for(unsigned int j = 0; j < other.getN(); ++j)
            {
                tmp_data[i * other.getN() + j] = *other[i * other.getN() + j];
            }
        }
        std::cout << "    Updated double* tmp_data" << std::endl;

        _m = other.getM();
        _n = other.getN();
        std::cout << "    Updated double _m and double _n" << std::endl;

        delete [] _data;
        std::cout << "    Freeed double* _data" << std::endl;
        _data = tmp_data;
        std::cout << "    Updated double* _data" << std::endl;
    }

    return *this;
}

double *Matrix::operator[](unsigned const int i)
{
    return &_data[i * _n];
}

Matrix Matrix::operator*(Matrix b)
{
    std::cout << "  Entered Matrix Matrix::operator*(Matrix b)" << std::endl;
    Matrix result(this->getM(), b.getN());
    std::cout << "  Created Matrix result" << std::endl;

    if(this->getN() != b.getM())
    {
        std::cerr << "Invalid Matrix size." << std::endl;
        throw std::logic_error("Invalid Matrix size.");
    }
    std::cout << "  Checked size of Input Matrize" << std::endl;

    for(unsigned int i = 0; i < this->getM(); ++i)
    {
        for(unsigned int j = 0; j < b.getN(); ++j)
        {
            double value = 0.0;
            for(unsigned int index = 0; index < b.getM(); ++index)
            {
                value += (*this)[i][index] * b[index][j];
            }
            result[i][j] = value;
        }
    }

    std::cout << "  Returning Matrix result" << std::endl;
    return result;
}

void Matrix::print()
{
    for(unsigned int i = 0; i < _m; ++i)
    {
        for(unsigned int j = 0; j < _n; ++j)
        {
            std::cout << (*this)[i][j];
            if(j < _n - 1)
            {
                std::cout << " | ";
            }
        }
        std::cout << std::endl;
    }
}

main.cpp:

#include "mat.h"
#include <exception>
#include <iostream>
#include <string>
#include <time.h>
#include <opencv2/core/core.hpp>
#include <opencv2/highgui/highgui.hpp>
#include <opencv2/imgproc/imgproc.hpp>
using namespace cv;

#define DEBUG
#define DEBUG2

std::string paths[9] =  {"D:\\Matrix\\Pictures\\05.jpg",
                         "D:\\Matrix\\Pictures\\11.jpg",
                         "D:\\Matrix\\Pictures\\03.jpg",
                         "D:\\Matrix\\Pictures\\04.jpg",
                         "D:\\Matrix\\Pictures\\02.jpg",
                         "D:\\Matrix\\Pictures\\06.jpg",
                         "D:\\Matrix\\Pictures\\07.jpg",
                         "D:\\Matrix\\Pictures\\08.jpg",
                         "D:\\Matrix\\Pictures\\10.jpg"};

double dRand(double dMin, double dMax)
{
    double d = (double)rand() / RAND_MAX;
    return dMin + d * (dMax - dMin);
}

int main()
{
    srand(time(NULL));

    Matrix A(35 * 3, 15 * 3);
    Matrix B(15 * 3, 1);
    Matrix C(35 * 3, 1);

    Matrix ReferenceValue(35 * 3, 1);

    for(unsigned int i = 0; i < 35 * 3; ++i)
    {
        for(unsigned int j = 0; j < 15 * 3; ++j)
        {
            A[i][j] = dRand(0.0, 0.02);
        }
    }

    namedWindow("Full Size Image", WINDOW_AUTOSIZE);

    namedWindow("Original Image Section", WINDOW_NORMAL);
    resizeWindow("Original Image Section", 7 * 50, 5 * 50);

    namedWindow("Downscaled Image Section", WINDOW_NORMAL);
    resizeWindow("Downscaled Image Section", 5 * 50, 3 * 50);

    namedWindow("Output", WINDOW_NORMAL);
    resizeWindow("Output", 7 * 50, 5 * 50);

    for(unsigned int index = 0; index < 9; ++index)
    {
        Mat image = imread(paths[index], CV_LOAD_IMAGE_COLOR);
        imshow("Full Size Image", image);

        Mat ToScaleDown(5, 7, image.type());

        std::cout << "Rows: " << image.rows << ", Cols: " << image.cols << std::endl;

        for(unsigned int x = 0; x < image.cols - 7; ++x)
        {
            for(unsigned int y = 0; y < image.rows - 5; ++y)
            {
                for(unsigned int xSection = 0; xSection < 7; ++xSection)
                {
                    for(unsigned int ySection = 0; ySection < 5; ++ySection)
                        {
                        //Updaten der ReferenceValue Matrix
                        }
                }

                std::cout << "Updated Matrix ReferenceValue" << std::endl;

                ToScaleDown = image(Range(y, y + 5), Range(x, x + 7));
                std::cout << "Loaded cv::Mat ToScaleDown" << std::endl;
                imshow("Original Image Section", ToScaleDown);
                resize(ToScaleDown, ToScaleDown, cvSize(5, 3), 0, 0, INTER_AREA);
                std::cout << "Resized cv::Mat ToScaleDown" << std::endl;
                imshow("Downscaled Image Section", ToScaleDown);

                for(unsigned int i = 0; i < 3; ++i)
                {
                    for(unsigned int j = 0; j < 5; ++j)
                    {
                        //Updaten der Matrix B
                    }
                }
                std::cout << "Updated Matrix B" << std::endl;

                C = A * B;
                std::cout << "Calculated Matrix C" << std::endl;

                Mat OutputMat(5, 7, image.type());
                for(unsigned int i = 0; i < 5; ++i)
                {
                    for(unsigned int j = 0; j < 7; ++j)
                    {
                        //Matrix C -> cv::Mat OutputMat
                    }
                }
                std::cout << "Generated cv::Mat OutputMat" << std::endl;
                imshow("Output", OutputMat);

                for(unsigned int i = 0; i < 35 * 3; ++i)
                {
                    //Bearbeiten der Matrix A
                }
                std::cout << "Matrix A Re-Calculated" << std::endl;
                std::cout << std::endl;
                cvWaitKey();
            }
            std::cout << "One Column - " << x << std::endl;
        }
    }
}

Das Log bist zum Absturz sieht dann wie folgt aus:

Entered Matrix::Matrix(unsigned const int m, unsigned const int n)
    Allocated double* _data
    Set values to 0
    Entered Matrix::Matrix(unsigned const int m, unsigned const int n)
    Allocated double* _data
    Set values to 0
    Entered Matrix::Matrix(unsigned const int m, unsigned const int n)
    Allocated double* _data
    Set values to 0
    Entered Matrix::Matrix(unsigned const int m, unsigned const int n)
    Allocated double* _data
    Set values to 0
    Entered Matrix::Matrix(unsigned const int m, unsigned const int n)
    Allocated double* _data
    Set values to 0
Rows: 747, Cols: 1023
Updated Matrix ReferenceValue
Loaded cv::Mat ToScaleDown
Resized cv::Mat ToScaleDown
Updated Matrix B
  Entered Matrix Matrix::operator*(Matrix b)
    Entered Matrix::Matrix(unsigned const int m, unsigned const int n)
    Allocated double* _data
    Set values to 0
  Created Matrix result
  Checked size of Input Matrize                     //Schlechtes Englisch ^^
  Returning Matrix result
    Allocated double* tmp_data
    Updated double* tmp_data
    Updated double _m and double _n
    Freeed double* _data
    Updated double* _data
Calculated Matrix C
Generated cv::Mat OutputMat
Matrix A Re-Calculated

Updated Matrix ReferenceValue
Loaded cv::Mat ToScaleDown
Resized cv::Mat ToScaleDown
Updated Matrix B
  Entered Matrix Matrix::operator*(Matrix b)
    Entered Matrix::Matrix(unsigned const int m, unsigned const int n)

MfG DragonRaider

Edit 1: Ich hatte mal gelesen, es wäre sauberer sich Argumente als const übergeben zu lassen, wenn man sie in der Funktion nicht verändern möchte
Edit 2: Danke, das mit memset hatte sich mir beim Lesen der Referenz auch schon erschlossen. Aber ich war einfach mal davon ausgegangen, dass man 0 halt mit nur Nullen "darstellt"
Edit 3: Hahaha in der main.cpp fehlte ein Stück ^^

[Furble Wurble]

Das ganze brauche ich mir gar nicht ansehen.
Der fehlende Kopierkonstruktor ist schon mal ein schöner showstopper.

Sobald Du eine Kopie anfertigst (z.b. das Argument von Matrix& operator=(Matrix); ), fliegt Dir das doch alles um die Ohren.

Schau Dir nochmal Referenzen als Typen von Funktionsargumenten an. Also "by value" vs. "by reference".

Lies mal, was man unter der Rule of Three in C++ versteht und warum Dich das betrifft.

[DragonRaider]

Vielen Dank ich habe mir den Artikel zur Rule of Three noch ein mal durch gelesen und daraufhin den Copykonstruktor und, soweit mir das logisch vor kam noch ein mal die const neu gesetzt. Jetzt sieht meine Klasse wie folgt aus:

mat.h:

#ifndef MATRIX_LIBRARY_H
#define MATRIX_LIBRARY_H

class Matrix
{
public:
    Matrix(unsigned int m, unsigned int n);                //m = rows, n = columns
    Matrix(const Matrix &other);
    ~Matrix();

    unsigned int getN() const;
    unsigned int getM() const;

    Matrix operator*(const Matrix b) const;
    double *operator[](unsigned const int& i) const;
    Matrix& operator=(Matrix other);

    void print();

private:
    unsigned int _n, _m;
    double *_data;
};

#endif

mat.cpp

#include "mat.h"
#include <stdexcept>
#include <cstdlib>
#include <cstring>
#include <iostream>

Matrix::Matrix(unsigned int m, unsigned int n) : _n(n), _m(m)
{
    _data = new double[_m * _n];
    memset(_data, 0, sizeof(double) * _m * _n);
}

Matrix::Matrix(const Matrix& other)
{
    double *tmp_data = new double[other.getM() * other.getN()];

    for(unsigned int i = 0; i < other.getM(); ++i)
    {
        for(unsigned int j = 0; j < other.getN(); ++j)
        {
            tmp_data[i * other.getN() + j] = *other[i * other.getN() + j];
        }
    }

    _m = other.getM();
    _n = other.getN();

    delete [] _data;
    _data = tmp_data;
}

Matrix::~Matrix()
{
    delete [] _data;
}

unsigned int Matrix::getN() const
{
    return _n;
}

unsigned int Matrix::getM() const
{
    return _m;
}

Matrix& Matrix::operator=(Matrix other)
{
    if(this != &other)
    {
        double *tmp_data = new double[other.getM() * other.getN()];

        for(unsigned int i = 0; i < other.getM(); ++i)
        {
            for(unsigned int j = 0; j < other.getN(); ++j)
            {
                tmp_data[i * other.getN() + j] = *other[i * other.getN() + j];
            }
        }

        _m = other.getM();
        _n = other.getN();

        delete [] _data;
        _data = tmp_data;
    }

    return *this;
}

double* Matrix::operator[](unsigned const int& i) const
{
    return &_data[i * _n];
}

Matrix Matrix::operator*(const Matrix b) const
{
    Matrix result(this->getM(), b.getN());

    if(this->getN() != b.getM())
    {
        std::cerr << "Invalid Matrix size." << std::endl;
        throw std::logic_error("Invalid Matrix size.");
    }

    for(unsigned int i = 0; i < this->getM(); ++i)
    {
        for(unsigned int j = 0; j < b.getN(); ++j)
        {
            double value = 0.0;
            for(unsigned int index = 0; index < b.getM(); ++index)
            {
                value += (*this)[i][index] * b[index][j];
            }
            result[i][j] = value;
        }
    }

    return result;
}

void Matrix::print()
{
    for(unsigned int i = 0; i < _m; ++i)
    {
        for(unsigned int j = 0; j < _n; ++j)
        {
            std::cout << (*this)[i][j];
            if(j < _n - 1)
            {
                std::cout << " | ";
            }
        }
        std::cout << std::endl;
    }
}

Jetzt funktioniert es... Dachte ich ^^ solange, bis ich die print()-Funktion wieder genutzt habe. Seitdem stürzt das Programm wieder beim 3. oder 4. mal der Benutzung des Operators * ab. Wenn ich sie wieder aus meinem Code entferne gibt es "keine" Probleme mehr(bestimmt, aber man merkt es auf jeden Fall nicht einfach so). Woran könnte das liegen
Danke noch mal,
MfG
DragonRaider

[Furble Wurble]

Dein Kopierkonstruktor sieht eher wie eine Zuweisung aus. Du brauchst gar keinen temporären Puffer und das delete[] _data; ist tödlich.

Das Interface gefällt mir sehr viel besser, da hast Du aufmerksam gelesen.

[C++ Forumbot]

Dieser Thread wurde von Moderator/in SeppJ aus dem Forum C (C89, C99 und C11) in das Forum C++ (auch C++0x und C++11) verschoben.

Im Zweifelsfall bitte auch folgende Hinweise beachten:
C/C++ Forum :: FAQ - Sonstiges :: Wohin mit meiner Frage?

Dieses Posting wurde automatisch erzeugt.

[manni66]

Und wenn man statt Zeiger, new/delte/memset und "Regel der Drei" bzw. "Regel der Fünf" einfach std::vector nimmt, wird es viel einfacher.

[DragonRaider]

Hallo,
@Furble Wurble: Freut mich, dass das Ganze jetzt besser ist ich habe die Funktion print() auch noch const gemacht(verändert ja auch nichts) wegen dem Kopierkonstruktor: Da hast Du Recht, da hatte ich nicht richtig drüber nachgedacht. Das hat mein Problem gelöst vielen Dank besonders an Dich, aber auch an alle Anderen die mir geholfen haben
@manni66: Du hast Recht, so ginge es natürlich. Aber so habe ich doch viel mehr gelernt, oder? Jetzt weiß ich (wennn bestimmt auch noch nicht perfekt), wann man const bei Memberfunktionen verwenden sollte und wie ein Zuweisungsoperator und ein Kopierkonstruktor so ungefähr auszusehen haben. Und ich kenne mehr mögliche Fehlerquellen. Das alles wird mir in Zukunft helfen, eigene Klassen zu schreiben, die vielleicht nicht ganz so simpel sind/sich nicht "umgehen" lassen.
MfG
DragonRaider

PS.: Vielen Dank auch für's Verschieben

[DragonRaider]

Hi,
ich bin's noch mal. Ich war mir nicht sicher, ob das wirklich in nen anderen Thread sollte... Darum hier Das Problem ist jetzt nämlich: Wenn ich den Zuweisungsoperator ganz pur nach dem Motto

A = B;

verwende, stürzt das Programm ab ^^
MfG
DragonRaider

[Marc-O]

Hallo

Ich würde dir folgende Artikel nochmal ans Herz legen: Überladung von Operatoren in C++ (Teil 1). Gerade wegen deinem Zuweisungsoperator.

Wegen deinem direkten Problem, solltest du deinem Kopiert-Konstruktor und deine Zuweisungs-Operator debuggen um zu sehen wo genau der Fehler auftritt.

Edit: Die Code-Zeile "delete [] _data" hast du hoffentlich aus dem Kopiekonstruktor entfernt. Zum anderen hast du innerhalb deines Kopiert-Konstruktors sowie auch im Zuweisungsoperator fehlerhafte Indexzugriffe ins Array, hier ein angepasster Zuweisungsoperator:

Matrix& Matrix::operator=(Matrix const& other)
{
  if (this != &other)
  {
    double *tmp_data = new double[other.getM() * other.getN()];

    for (unsigned int i = 0; i < other.getM(); ++i)
      for (unsigned int j = 0; j < other.getN(); ++j)
        // i muss mit getN multipliziert werden
        tmp_data[i * other.getN() + j] = other[i][j]; 
        // Beim zugriff auf _other_ wird nicht direkt ins _data gegriffen, sondern auf deinen 
        // ueberladenen Operator, den du implementiert hast. Ansonst koenntest du es noch so machen:
        //tmp_data[i * other.getN() + j] = other._data[i * other._n + j];

    _m = other.getM();
    _n = other.getN();

    delete[] _data;
    _data = tmp_data;
  }

  return *this;
}

Mfg mdn

[camper]

tmp_data[i * other.getN() + j] = *other[i * other.getN() + j];

die rechte Seite sieht seltsam aus...

[Furble Wurble]

Ich finde den operator[] , der einen double* zurückgibt äusserst suspekt.

Der klassische Rat ist einen double& operator()(int x, int y); zu implementieren und auf operator[] zu verzichten.

Ansonsten ist es recht schwierig einen Fehler zu erkennen, wenn Du nicht die aktuelle Implementierung postest.

[DragonRaider]

Hallo,
ich habe mich bemüht, schon einmal auf eure Einwände ein zu gehen. Im Code ist deshalb auch einiges Kommentiert.
@Marc-O: _data ist private, also ist direkter zugriff nicht möglich(oder verwechsle ich da etwas?).

mat.h

#ifndef MATRIX_LIBRARY_H
#define MATRIX_LIBRARY_H

class Matrix
{
public:
    Matrix(unsigned int m, unsigned int n);                //m = rows, n = columns
    Matrix(const Matrix &other);
    ~Matrix();

    unsigned int getN() const;
    unsigned int getM() const;

    Matrix operator*(const Matrix b) const;
    double* operator[](unsigned const int& i) const;
    double& operator()(unsigned int i, unsigned int j);
    Matrix& operator=(const Matrix other);

    void print() const;

private:
    unsigned int _n, _m;
    double *_data;
};

#endif

mat.cpp

#include "mat.h"
#include <stdexcept>
#include <cstdlib>
#include <cstring>
#include <iostream>

Matrix::Matrix(unsigned int m, unsigned int n) : _n(n), _m(m)
{
    _data = new double[_m * _n];
    memset(_data, 0, sizeof(double) * _m * _n);
}

Matrix::Matrix(const Matrix& other)
{
    _data = new double[other.getM() * other.getN()];

    /*for(unsigned int i = 0; i < other.getM(); ++i)
    {
        for(unsigned int j = 0; j < other.getN(); ++j)
        {
            _data[i * other.getN() + j] = *other[i * other.getN() + j];
        }
    }*/
    for(unsigned int i = 0; i < other.getM() * other.getN(); ++i)
    {
        _data[i] = other[i][0];                                                                //Besser verständlich?
    }

    _m = other.getM();
    _n = other.getN();
}

Matrix::~Matrix()
{
    delete [] _data;
}

unsigned int Matrix::getN() const
{
    return _n;
}

unsigned int Matrix::getM() const
{
    return _m;
}

Matrix& Matrix::operator=(const Matrix other)
{
    if(this != &other)
    {
        double *tmp_data = new double[other.getM() * other.getN()];

        /*for(unsigned int i = 0; i < other.getM(); ++i)
        {
            for(unsigned int j = 0; j < other.getN(); ++j)
            {
                tmp_data[i * other.getN() + j] = *other[i * other.getN() + j];                   //Der Operator[] gibt die Addresse mit einem "Offset" zurück, sodass man die von **Arrays gewohnten Dinge machen kann.
            }                                                                                    //*other[i * other.getN() + j] ist somit das gleiche wie other[i * other.getN() + j][0]
        }*/
                                                                                                 //Jedoch ist diese Form sowieso ineffizient
        for(unsigned int i = 0; i < other.getM() * other.getN(); ++i)
        {
            tmp_data[i] = other[i][0];                                                           //Besser verständlich?
        }

        _m = other.getM();
        _n = other.getN();

        delete [] _data;
        _data = tmp_data;
    }

    return *this;
}

double* Matrix::operator[](unsigned const int& i) const                                          //Meiner Meinung nach intuitiver, da es sich verhält wie ein double** Array
{
    return &_data[i * _n];
}

double& Matrix::operator()(unsigned int i, unsigned int j)                                       //Auch gut ^^
{
    return _data[i * _n + j];
}

Matrix Matrix::operator*(const Matrix b) const
{
    Matrix result(this->getM(), b.getN());

    if(this->getN() != b.getM())
    {
        std::cerr << "Invalid Matrix size." << std::endl;
        throw std::logic_error("Invalid Matrix size.");
    }

    for(unsigned int i = 0; i < this->getM(); ++i)
    {
        for(unsigned int j = 0; j < b.getN(); ++j)
        {
            double value = 0.0;
            for(unsigned int index = 0; index < b.getM(); ++index)
            {
                value += (*this)[i][index] * b[index][j];
            }
            result[i][j] = value;
        }
    }

    return result;
}

void Matrix::print() const
{
    for(unsigned int i = 0; i < _m; ++i)
    {
        for(unsigned int j = 0; j < _n; ++j)
        {
            std::cout << (*this)[i][j];
            if(j < _n - 1)
            {
                std::cout << " | ";
            }
        }
        std::cout << std::endl;
    }
}

MfG
DragonRaider

[Belli]

Du kannst Dir übrigens in Deinem Kopierkonstruktor den Aufruf der get-Methoden sparen:

//_m = other.getM();
_m = other._m; //usw.