what else is wrong ??

Hi, I'm very close to finish my last planned console program, which multiplies two matrices, but i'm having unhandled exception :( I check my code million of times but i dont know what's wrong :(

can anyone plz take a look ??
btw, this is NOT home work :D i'm just doing that for fun and increase the skills :)

header file :

Code:

---

#ifndef MATRIX_H
#define MATRIX_H


class Matrix
{
      //Data members
      private:
              int m_Rows;     
              int m_Columns;
              float** m_pMatrice;
         
           
      //Member functions     
      public:
            //Constructor               
              Matrix::Matrix();
           
           
            //Destructor
            ~Matrix();
           
         
            //Building function
            void build(int& rows, int& columns);
         
            //Overloading the multiplication operator
            Matrix operator*(const Matrix& aMatrix);
         
                        //Resize result
                        void resizeResult(int rows, int columns);
           
};

#endif

---

implementation file :

Code:

---

#include "stdafx.h"
#include <iostream>
#include "matrix.h"

using namespace std;

//Contructor
Matrix::Matrix(): m_pMatrice(0), m_Rows(0), m_Columns(0)
{
                  //Empty constructor body
}

//Destructor
Matrix::~Matrix()
{
                                       
                                                for(int i = 0; i < m_Rows; ++i)
                                                        delete[] m_pMatrice[i]; // delete array for each row
                                                        delete[] m_pMatrice; // delete column of pointers
}

//Building matrice
void Matrix::build(int& rows, int& columns)
{
               
                    m_Rows = rows;
                    m_Columns = columns;
                 
                 
               
                    cout << endl;
                    cout << "You have made a matrice with the will\n";
                    cout << "of Mr.Grant, the God of Math.\n";
                    cout << "[" << m_Rows << "][" << m_Columns << "]\n";
                 
                 
                    m_pMatrice = new float*[m_Rows]; // allocate m pointers
                    for(int i = 0; i < m_Rows; ++i)
                    m_pMatrice[i] = new float[m_Columns]; // allocate n columns for each row
                 
                    cout << endl;
                    cout << "Now, please pay attention, we will fill up" << endl;
                    cout << "the entries of your matrice." << endl;
                    for(int i = 0; i < m_Rows; ++i)
                    {
                          for(int j = 0; j < m_Columns; ++j)
                          {
                            cout << "entry[" << i << "][" << j << "] = ";
                            cin >> m_pMatrice[i][j];
                          }
                    }
}


//Resizing result
void Matrix::resizeResult(int rows, int columns)
{
        // create the new matrix
    this->m_pMatrice = new float*[rows];
    for(int i = 0; i < rows; ++i)
          this->m_pMatrice[i] = new float[columns];
        this->m_Rows = rows;
        this->m_Columns = columns;
}

//Multiplying matrices
Matrix Matrix::operator*(const Matrix& rhs)
{
      Matrix result;
   
     
   
      if (this == &rhs)
      {
                return *this;
      }
      else
      {
            //Checks if matrices are able to be multiplied
            if (this->m_Columns == rhs.m_Rows)
            {
                //Do multiplication
                                result.resizeResult(this->m_Rows, rhs.m_Columns);
                                for(int i = 0; i < result.m_Rows; ++i)
                                for(int j = 0; j < result.m_Columns; ++j)

                                                                              //Error occurs here, "Unhandled exception"
                                        result.m_pMatrice[i][j] = (float)i*rhs.m_Columns+j;               
            }
            else
            {
                cout << "Cannot perform multiplication." << endl;
                cout << "Columns of first matrix don't ";
                cout << "match the rows of the second matrix" << endl;
            }
      }
   
      return result;
}

---

application code :

Code:

---

//Special thanks to Frank Luna who helped me :)

#include "stdafx.h"
#include <iostream>
#include <conio.h>
#include "matrix.h"

using namespace std;




int main()
{
    cout << "\tMatrices Multiplication 1.0 Beta" << endl;
    cout << "\tDedicated specially with gratitude to my math teacher: " << endl;
    cout << "\tMr.Grant." << endl << endl;
    int rows, columns;
    //////////////////////////////////////////////////////
    ////////////////Creating Matrice1/////////////////////
    Matrix mtrx1;
 
 
 
    cout << "Enter the rows of the first matrice: "; cin >> rows;
    cout << endl;
 
    cout << "Enter the columns of the first matrice: ";cin >> columns;
    mtrx1.build(rows, columns);
    cout << endl;
    //////////////////////////////////////////////////////
 
    //////////////Creating Matrice2///////////////////////
    Matrix mtrx2;
 
 
    cout << "Enter the rows of the second matrice: "; cin >> rows;
    cout << endl;
 
    cout << "Enter the columns of the second matrice: "; cin >> columns;
    mtrx2.build(rows, columns);
    //////////////////////////////////////////////////////
 
    ////////////////////////////////////////////////////////
    //////////////Multiplication process////////////////////
    Matrix result;
    result = mtrx1 * mtrx2;
    ////////////////////////////////////////////////////////
 
 
 
    //Cleaning allocated memory
    mtrx1.~Matrix();
    mtrx2.~Matrix();
        result.~Matrix();

    getch();
    return 0;
}

---

please add a comment in one of the code segments at the line where it says the error occurs.

im not overly familiar with C++, but i didnt think you could call the destructor explicitly.

Code:

---

    //Cleaning allocated memory
    mtrx1.~Matrix();
    mtrx2.~Matrix();
        result.~Matrix();

---

in your build function is it necessary to pass the parameters by reference?

i added the comment where the error occurs, it's in the implementation file, in the overloaded * function


even when i remove the very last line (of cleaning allocated memory) the same occurs. They dont have nothing to say about that error

have you tried printing out mtrx1 and mtrx2 before doing the * to verify its contents?

edit: could it have something to do with how your always pre-incrementing your counters rather than post-incrementing? if i were you i would debug by changing them all to the latter.

also this cast i believe will only cast the first variable (at least thats how it is in java!), if you want it all to be casted then put parenthesis around all of the arithmetic.. although im sure this wouldnt cause an 'unhandled exception':

Code:

---

(float)i*rhs.m_Columns+j;

---

i just ran it and it doesnt give me any errors. i used two 2*2 matrices and gave values 1-8. it crashes with 1*1 matrix though, due to your preincrements.

>> even when i remove the very last line (of cleaning allocated memory) the same occurs.
That's fine, you should still leave it out. The destructors are called automatically, and will be called twice if you do it yourself.

Since you are using dynamic arrays instead of vector, you have to write a copy constructor and an assignment operator for your matrix class. If you don't, then when the operator* returns the matrix by value, it will not copy to the result correctly (you will delete the data that result is still using).

On the line you say has the exception, the cast is not necessary at all. However, you aren't multiplying the values in the matrices, are you sure that's what you wanted. It shouldn't crash, but it shouldn't give the correct values.

>> it crashes with 1*1 matrix though, due to your preincrements.
Preincrements in a for loop are the same as post-increments. That should not cause a crash.

I don't know why the crash is occurring where the comment in the code is, but add the copy constructor and assignment operator anyway, since those are definitely required to stop a crash from occurring. Maybe that is the actual problem.

Quote:

---

Preincrements in a for loop are the same as post-increments. That should not cause a crash.

---

well i learned something! i guess it works that way only in while loops. thanks for the catch.

Quote:

---

i just ran it and it doesnt give me any errors. i used two 2*2 matrices and gave values 1-8. it crashes with 1*1 matrix though, due to your preincrements.

---

strange, i still get the undandled exception although i changed it to post increment

==========================
i added the copy constructor, and assignment operator, the program crashes:

Code:

---

//Copy constructor
Matrix::Matrix(const Matrix& m)
{
                                        m_pMatrice = 0;
                                        *this = m;
}

---

Code:

---

Matrix& Matrix::operator =(const Matrix &rhs)
{
        if (this == &rhs)
        {
                return *this;
        }
        else
        {
                m_pMatrice = rhs.m_pMatrice;
                return *this;
        }

}

---

any help :(

From what I see, each Matrix owns its 2D array of floats. As such, you are not implementing your destructor, copy constructor, and copy assignment properly.

Since you new[] a pointer of pointers, you need to delete[] that pointer.

I suggest implementing your copy assignment operator in terms of your constructor and a swap function. For example:

Code:

---

Matrix& Matrix::operator=(const Matrix& rhs)
{
    if (this != &rhs)
    {
        Matrix temp(rhs);
        swap(temp);
    }
    return *this;
}

void Matrix::swap(Matrix& matrix)
{
    using std::swap;
    swap(m_Rows, matrix.m_Rows);
    swap(m_Columns, matrix.m_Columns);
    swap(m_pMatrice, matrix.m_pMatrice);
}

---

Your copy constructor cannot just copy the pointer to the array of arrays of floats. If it does that (as it currently does, via the copy assignment operator), then when a copy goes out of scope, it delete[]s the array of arrays of the original Matrix as well. Rather, you need to loop through the array of arrays and copy each float to the new Matrix object.

Incidentally, since you do not intend to change the arguments passed to your build() member function, those arguments should be passed either by value or by const reference. Also, since your matrix multiplication does not change the current Matrix, you probably should either declare operator* as const, or make it a free function.