Problem with a class that stores 2 dimensional matrix

sys7em · 06-08-2009, 01:01 PM

Hello guys,

I have this code which stores 2d matrix in a class matrix (keeping its elements in a dynamically allocated array of integers (int **A

However when I execute the program it reads the matrix and outputs it correctly but when it's being multiplied by a scalar the program crashes.

Why is this ?

Here's my code:

Code:

#include<iostream>
#include <vector>
#include <ctime>
#include <fstream>
#include <algorithm>
#include <string>
#include <cmath>
#include <cstdlib>
#include <ctime>
#include <numeric>
#include <iterator>
#include <iomanip>

class matrix
{
	int **A;                          // pointer to store 2 dimensional array
	int x, y;

public:
	matrix();

	matrix(int, int);
	
	void read();
	void input();
   	void display();
	matrix operator* (int);
};

//
matrix :: matrix()
{
	x = 0;
	y = 0;
}

matrix :: matrix(int a, int b)
{
	x = a;
	y = b;
}



//read matrix
void matrix :: read()
{
	std::cout << " enter number of rows " << std::endl;
	std::cin >> x;
	std::cout << " enter number of columns " << std::endl;
	std::cin >> y;
}

//matrix input
void matrix :: input()
{ 
    	int num1, num2, k;
     	int i, j;
	A = new int *[x];            
	for (k = 0; k < x; k++)     	
		A[k] = new int [y];
	for (num1=0; num1<x; num1++)
	{

		for (num2=0; num2<y; num2++)
		{
			A[num1][num2] = 0;
		}
	} 
	for ( i = 0; i < x; i++)
	{
		for ( j = 0; j < y; j++)
		{
			std::cin >> A[i][j];
     	}

	}

}


//matrix display
void matrix :: display()

{
    	int i, j;
	for (i=0; i<x; i++)
	{
		for (j=0; j<y; j++)
		{
			std::cout << "  " << A[i][j];
		}
		std::cout << "\n";
	}
} 


//matrix mult
matrix  matrix :: operator* (int a)
{
	matrix temp(x, y);
	int num,num1, num2;
        for (num1=0; num1<temp.x; num1++)
	{
		for (num2=0; num2<temp.y; num2++)
		{
			temp.A[num1][num2] = 0;
		}
	}
	int i, j;
	for ( i = 0; i < x; i++)
	{
		for ( j = 0; j < y; j++)
		{
			temp.A[i][j] = a * A[i][j];
		}
	}
	return (temp);
}


// MAIN
int main ()

{

matrix s;
        matrix t = s;

	matrix M1, M2;
	M1.read ();
    	M1.input ();
 	M1.display ();
    	M2 = M1*2;
	M2.display();

	return 0;

}

Sergei Steshenko · 06-08-2009, 01:41 PM

Quote:

Originally Posted by sys7em

Hello guys,

I have this code which stores 2d matrix in a class matrix (keeping its elements in a dynamically allocated array of integers (int **A

However when I execute the program it reads the matrix and outputs it correctly but when it's being multiplied by a scalar the program crashes.

Why is this ?

Here's my code:

Code:

#include<iostream>
#include <vector>
#include <ctime>
#include <fstream>
#include <algorithm>
#include <string>
#include <cmath>
#include <cstdlib>
#include <ctime>
#include <numeric>
#include <iterator>
#include <iomanip>

class matrix
{
	int **A;                          // pointer to store 2 dimensional array
	int x, y;

public:
	matrix();

	matrix(int, int);
	
	void read();
	void input();
   	void display();
	matrix operator* (int);
};

//
matrix :: matrix()
{
	x = 0;
	y = 0;
}

matrix :: matrix(int a, int b)
{
	x = a;
	y = b;
}



//read matrix
void matrix :: read()
{
	std::cout << " enter number of rows " << std::endl;
	std::cin >> x;
	std::cout << " enter number of columns " << std::endl;
	std::cin >> y;
}

//matrix input
void matrix :: input()
{ 
    	int num1, num2, k;
     	int i, j;
	A = new int *[x];            
	for (k = 0; k < x; k++)     	
		A[k] = new int [y];
	for (num1=0; num1<x; num1++)
	{

		for (num2=0; num2<y; num2++)
		{
			A[num1][num2] = 0;
		}
	} 
	for ( i = 0; i < x; i++)
	{
		for ( j = 0; j < y; j++)
		{
			std::cin >> A[i][j];
     	}

	}

}


//matrix display
void matrix :: display()

{
    	int i, j;
	for (i=0; i<x; i++)
	{
		for (j=0; j<y; j++)
		{
			std::cout << "  " << A[i][j];
		}
		std::cout << "\n";
	}
} 


//matrix mult
matrix  matrix :: operator* (int a)
{
	matrix temp(x, y);
	int num,num1, num2;
        for (num1=0; num1<temp.x; num1++)
	{
		for (num2=0; num2<temp.y; num2++)
		{
			temp.A[num1][num2] = 0;
		}
	}
	int i, j;
	for ( i = 0; i < x; i++)
	{
		for ( j = 0; j < y; j++)
		{
			temp.A[i][j] = a * A[i][j];
		}
	}
	return (temp);
}


// MAIN
int main ()

{

matrix s;
        matrix t = s;

	matrix M1, M2;
	M1.read ();
    	M1.input ();
 	M1.display ();
    	M2 = M1*2;
	M2.display();

	return 0;

}

And where do you allocate space for the array that is supposed to store matrix values ?

I mean, in "matrix matrix :: operator* (int a)" you have

Code:

matrix temp(x, y);

, but, if I understand C++ correctly, it doesn't allocate memory.

I do not understand your design in general - why do you allocate memory in "matrix :: input()" rather than in the constructor(s) ?

johnsfine · 06-08-2009, 02:35 PM

Quote:

Originally Posted by Sergei Steshenko

where do you allocate space for the array that is supposed to store matrix values ?

That is the bug, but I'm not sure Sergei explained it well enough for sys7em.

The following code to allocate space is only in matrix::input().

Code:

	A = new int *[x];            
	for (k = 0; k < x; k++)     	
		A[k] = new int [y];

You've kind of painted yourself into a corner with the overall design.

When you construct the object, you don't allocate the space, because for most of the objects you don't know what space to allocate at construction time.

When you copy the object, you don't allocate space for the copy nor duplicate the underlying data. Instead you just copy the pointer. That would work for simple examples, but means you won't know when to deallocate the space.

The actual crash, as Sergei described, happens when you try to use the space that should have been allocated for the object temp in matrix::operator*
But I don't see any clean way to correct it, because the implicit operator=() would still be losing ownership info. The implicit destructor would still be leaking memory and simply correcting that leak would cause crashes elsewhere.

You need to make an overall plan for allocating and deallocating the memory. To do that decently would involve duplicating a significant fraction of std::vector, which begs the question of why you didn't simply use std::vector