Matrix Multiplication with operator overloading

16,922

Solution 1

As I suspected, your copy constructor and assignment operator are in fact not implemented correctly. You are simply copying the pointer over. That means that when you copy one matrix to another, they both share the same data. When one of them goes out of scope, the destructor is called, then the shared data is deleted, leaving the remaining matrix with dangling pointers.

Fix those functions so they actually allocate new arrays, and copy the data.

Solution 2

  1. i actually goes from 0 to this->rows - 2 (because of i < n-1 for i = n-1 is false). Same for other loops. This seems not to be correct behaviour for matrices multiplication.
  2. Nevertheless, this code fragment seems to be correct. Can you provide us with full class implementation?

P.S. If T is type of matrix elements, then type of sum_elems should be T.

Share:
16,922
Admin
Author by

Admin

Updated on June 04, 2022

Comments

  • Admin
    Admin almost 2 years

    I am attempting to create an overloaded operator for a matrix class that I have built. My matrix class stores the matrix in a dynamically allocated multidimensional array. I am simply trying to test my overloaded operator by multiplying two matrices together that are exactly the same and display the output. I am getting weird results and I believe it has to do with one of the conditionals on my for loops. I have however walked through all of my for loops and can find nothing wrong. The matrices I am multiplying together are both 6x6.

    My overloaded operator

    template <typename T>
    const matrix<T> matrix<T>::operator * (const matrix& right) const
    {
        matrix<T> c = right;
        int sum_elems;
        for( int i = 0; i < this->rows - 1; ++i)
        {
            for(int j = 0; j < right.cols - 1; ++j)
            {
                sum_elems = 0;
                for( int k = 0; k < right.rows - 1; ++k)
                {
                    sum_elems += this->the_matrix[i][k] * right.the_matrix[k][j];
                }
    
                c.the_matrix[i][j] = sum_elems;
            }
        }
        return c;
    }                 
    

    Now my call to the overloaded operator in my main function:

    std::cout << my_matrix;
    matrix<int> copy_matrix;
    copy_matrix = my_matrix * my_matrix; 
    std::cout << copy_matrix;
    

    My output:

      The Matrix:
     0  1  0  1  1  0
     1  0  1  0  1  1
     0  1  0  1  0  1
     1  0  1  0  1  0
     1  1  0  1  0  1
     0  1  1  0  1  0
       The Matrix:
     -1  33  139587680  18  38  75
     139587680  18  38  75  157  1
     139587712  38  1470  4365  10411  1
     139587744  75  4365  19058932  64514866  0
     139587776  157  10411  64514866  1136204102  1
     139596144  1  1  0  1  0
    

    As you can see it seems that I am stepping out of bounds on one of my arrays. I can not seem to find where though. I appreciate your help in advance.

    Edit: As requested my full implementation of my matrix class

    Matrix Definitions:

    template <typename T>
    class matrix
    {
        public:
    
            //Default Constructor
            matrix();
    
            //Overloaded Constructor
            matrix(std::ifstream&, const char*);
    
            //Copy Constructor
            matrix(const matrix&);
    
            //Destructor
            ~matrix();
    
    
            //overloaded operators
            T* operator [] (T);
            const matrix operator * (const matrix&) const;
            matrix& operator = (const matrix&);
            friend std::ostream& operator << <T> (std::ostream&, const matrix<T>&); 
    
    
        private:
            T** the_matrix;
            unsigned rows, cols;
    

    Matrix Implementation:

    /* Template version of matrix class */
    /*---------------------------------------------------------------------------*/
    // Default contructor
    template <typename T>
    matrix<T>::matrix() { }
    
    // Overloaded contructor
    template <typename T>
    matrix<T>::matrix( std::ifstream& in, const char* file)
    {
    
        // declare the variables to be used
        T vertices, edges, u, v;
        std::string line;
    
        // open file for reading
        in.open(file);
    
        // get number of vertices
        in >> vertices;
    
    
        // throw away second line   
        std::getline(in, line);
        std::getline(in, line);
    
        // get number of edges and dump them in two arrays
        in >> edges;
        T edge1 [edges];
        T edge2 [edges];
        int j = 0, k = 0;
        for(int a = 0; a < edges; ++a)
        {    
            in >> u >> v;
            edge1[j] = u;
            edge2[k] = v;
            ++j;
            ++k;
        }
    
        in.close();
    
        // Create multi-dim-dynamic array
        rows = vertices;
        cols = vertices;
    
        the_matrix = new T*[rows];
    
        for( int b = 0; b < rows; ++b)
        {
            the_matrix[b] = new T [rows];
        }
    
        // Initialize array values to zero
        for ( int c = 0; c < rows; ++c)
        {
           for( int d = 0; d < cols; ++d)
           {
               the_matrix[c][d] = 0;
           }
        }
    
        // push the edges to the matrix
        for( int e = 0; e < edges; ++e)
        {
            the_matrix[edge1[e] - 1][edge2[e] - 1] = 1;
        }
        for ( int f = 0; f < edges; ++f)
        {
            the_matrix[edge2[f] - 1][edge1[f]-1] = 1;
        }
    
    
    }
    
    // Copy Constructor
    template <typename T>
    matrix<T>::matrix(const matrix& left)
    {
        the_matrix = left.the_matrix;
        rows = left.rows;
        cols = left.cols;
        spath = left.spath;
    }
    
    // Destructor
    template <typename T>
    matrix<T>::~matrix()
    { 
        // Deletes the data in reverse order of allocation
        for( int a = cols; a > 0; --a)
        {
            delete[ ] the_matrix[a];
        }
    
        delete[ ] the_matrix;
    }
    
    // Overloaded * Operator
    template <typename T>
    const matrix<T> matrix<T>::operator * (const matrix& right) const
    {
        matrix<T> c = right;
        T sum_elems;
        for( int i = 0; i < this->rows - 1; ++i)
        {
            for(int j = 0; j < right.cols - 1; ++j)
            {
                sum_elems = 0;
                for( int k = 0; k < right.rows - 1; ++k)
                {
                    sum_elems += this->the_matrix[i][k] * right.the_matrix[k][j];
                }
    
                c.the_matrix[i][j] = sum_elems;
            }
        }
        return c;
    }
    
    // Overloaded assignment Operator
    template <typename T>
    matrix<T>& matrix<T>::operator = (const matrix& right)
    {
        this->the_matrix= right.the_matrix;
        this->rows = right.rows;
        this->cols = right.cols;
        this->spath = right.spath;
        return *this;
    }
    
    // Overloaded << operator
    template <typename T>
    std::ostream& operator << (std::ostream& output, const matrix<T>& left)
    {
        // Test screen output to see if correct   
        std::cout << std::setw(14) << "The Matrix:" << '\n';
        for( int a = 0; a < left.rows; ++a)
        {
            for( int b = 0; b < left.cols; ++b)
            {
                std::cout << ' ' << left.the_matrix[a][b] << ' ';
            }
            std::cout << '\n';
        }
        return output;
    }