[lnkForumImage]
TotalShareware - Download Free Software

Confronta i prezzi di migliaia di prodotti.
Asp Forum
 Home | Login | Register | Search 


 

Forums >

comp.lang.c++

segmentation fault overloaded operators

jr.freester

11/5/2008 6:29:00 PM

I have created to classes Matrix and System. System is made up of
type matrix.
----------------------------------------------------------------------------------
class Matrix
{
private:
int row, col;
double *data
public:
Matrix(const int& M, const int& N): row(M), col(N)
{
data = new double[row*col];
for(int m = 0; m<row; m++)
{
for(int n = 0; n<col; n++)
{
data[m*row + n] = 0;
}

}

...... etc
};

----------------------------------------------------------------------------------
class System
{
private:
int num_eq; //number of differential equations in
system
Matrix *equations; //array of class Matrix
public:
/*Constructor*/
System(const unsigned int num):num_eq(num)
{
equations = new Matrix[num_eq];
}
..... etc
};

In both classes I have overloaded an operator to facilitate addition.

----------------------------------------------------------------------------------
/*Addition Operator Matrix plus Matrix*/

friend Matrix operator +(const Matrix& A, const Matrix& B)
{

Matrix C(A.row, A.col);
assert((A.row == B.row) && (A.col == B.col));


for(int m = 0; m<C.row; m++)
{
for(int n = 0; n<C.col; n++)
{
C.data[m*C.col + n] = A.data[m*A.col + n] +
B.data[m*B.col + n];
}
}



return C;
}

----------------------------------------------------------------------------------
/*Addition Operator System plus System*/

friend System operator +(const System& A, const System& B)
{

System C(A.num_eq);
assert(A.num_eq == B.num_eq);
for(int i = 0; i<C.num_eq; i++)
{
C.equations[i] = A.equations[i] + B.equations[i];
}
return C;
}

I have written a simple driver program to test the functionality of
these two classes. Given the following variables which have been
properly initialized

Matrix A, B, C, D;
System W, X, Y, Z;

D = A + B + C; //produces valid results
Z = W + X; //produces valid results
Z = W + X + Y; //produces a segmentation fault

I have tried enclosing the terms within parenthesis but that does not
do anything. The code for the operator+ is nearly identical for both
classes. What causes the segmentation fault and how can I resolve it?


Justin

PS How do I inset code and preserve formatting?











3 Answers

Victor Bazarov

11/5/2008 7:37:00 PM

0

jr.freester@gmail.com wrote:
> I have created to classes Matrix and System. System is made up of
> type matrix.
> ----------------------------------------------------------------------------------
> class Matrix
> {
> private:
> int row, col;
> double *data
^^^
A semicolon is missing here...

> public:
> Matrix(const int& M, const int& N): row(M), col(N)
> {
> data = new double[row*col];
> for(int m = 0; m<row; m++)
> {
> for(int n = 0; n<col; n++)
> {
> data[m*row + n] = 0;
> }

A curly brace seems missing here...

>
> }

You can shorten this significantly if you just write

Matrix(const int& M, const int& N)
: row(M), col(N), data = new double[row*col]() {}

(notice the parentheses after the bracketed expression).


>
> ..... etc

Well, it is very important *what* you have here. Have you followed the
"Rule of Three" carefully?

> };
>
> ----------------------------------------------------------------------------------
> class System
> {
> private:
> int num_eq; //number of differential equations in
> system
> Matrix *equations; //array of class Matrix
> public:
> /*Constructor*/
^^^^^^^^^^^^^^^
Sorry, this comment is bogus.

> System(const unsigned int num):num_eq(num)
> {
> equations = new Matrix[num_eq];

Again, consider initialisation instead of assigning.

> }
> .... etc

Again, the question here is whether the Rule of Three was followed. Of
course, there is another way - don't use dynamically allocated manual
arrays for the data in 'Matrix' and 'System'. Use 'vector<double>' and
'vector<Matrix>' instead.

> };
>
> In both classes I have overloaded an operator to facilitate addition.
>
> ----------------------------------------------------------------------------------
> /*Addition Operator Matrix plus Matrix*/
>
> friend Matrix operator +(const Matrix& A, const Matrix& B)
> {
>
> Matrix C(A.row, A.col);
> assert((A.row == B.row) && (A.col == B.col));

I would rewrite it slightly. Assert first, only then allocate. Don't
allocate the default, copy the 'A' matrix there. Once you copied, use
the compound assignment instead of regular one.

Do

Matrix C(A); // copy-initialisation

instead

>
>
> for(int m = 0; m<C.row; m++)
> {
> for(int n = 0; n<C.col; n++)
> {
> C.data[m*C.col + n] = A.data[m*A.col + n] +
> B.data[m*B.col + n];

Do

C.data[..] += B.data[..];

instead.

> }
> }
>
>
>
> return C;
> }

If you created all necessary pieces to handle copying, you should have
no problem.

>
> ----------------------------------------------------------------------------------
> /*Addition Operator System plus System*/
>
> friend System operator +(const System& A, const System& B)
> {
>
> System C(A.num_eq);
> assert(A.num_eq == B.num_eq);
> for(int i = 0; i<C.num_eq; i++)
> {
> C.equations[i] = A.equations[i] + B.equations[i];
> }
> return C;
> }
>
> I have written a simple driver program to test the functionality of
> these two classes. Given the following variables which have been
> properly initialized
>
> Matrix A, B, C, D;

There is no default constructor in your Matrix class. Hence I have a
problem with this code of yours. It's not the code you have. See the
FAQ 5.8.

> System W, X, Y, Z;
>
> D = A + B + C; //produces valid results
> Z = W + X; //produces valid results
> Z = W + X + Y; //produces a segmentation fault
>
> I have tried enclosing the terms within parenthesis but that does not
> do anything. The code for the operator+ is nearly identical for both
> classes. What causes the segmentation fault and how can I resolve it?
>
>
> Justin
>
> PS How do I inset code and preserve formatting?

Make sure you have spaces instead of tabs. If you have, I'm not sure
what problem you are experiencing.

All in all, I would say RTFFAQ first, and then ask other questions.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

jr.freester

11/5/2008 10:00:00 PM

0

On Nov 5, 2:36 pm, Victor Bazarov <v.Abaza...@comAcast.net> wrote:
> jr.frees...@gmail.com wrote:
> > I have created to classes Matrix and System.  System is made up of
> > type matrix.
> > ---------------------------------------------------------------------------­-------
> > class Matrix
> > {
> >      private:
> >                  int row, col;
> >                  double *data
>
>                                ^^^
> A semicolon is missing here...
>
> >      public:
> >      Matrix(const int& M, const int& N): row(M), col(N)
> >      {
> >           data = new double[row*col];
> >           for(int m = 0; m<row; m++)
> >           {
> >               for(int n = 0; n<col; n++)
> >               {
> >                    data[m*row + n] = 0;
> >               }
>
> A curly brace seems missing here...
>
>
>
> >     }
>
> You can shorten this significantly if you just write
>
>     Matrix(const int& M, const int& N)
>       : row(M), col(N), data = new double[row*col]() {}
>
> (notice the parentheses after the bracketed expression).
>
>
>
> > ..... etc
>
> Well, it is very important *what* you have here.  Have you followed the
> "Rule of Three" carefully?
>
> > };
>
> > ---------------------------------------------------------------------------­-------
> > class System
> > {
> >     private:
> >         int num_eq;         //number of differential equations in
> > system
> >         Matrix *equations;   //array of class Matrix
> >     public:
> >         /*Constructor*/
>
>            ^^^^^^^^^^^^^^^
> Sorry, this comment is bogus.
>
> >         System(const unsigned int num):num_eq(num)
> >         {
> >             equations = new Matrix[num_eq];
>
> Again, consider initialisation instead of assigning.
>
> >         }
> > .... etc
>
> Again, the question here is whether the Rule of Three was followed.  Of
> course, there is another way - don't use dynamically allocated manual
> arrays for the data in 'Matrix' and 'System'.  Use 'vector<double>' and
> 'vector<Matrix>' instead.
>
> > };
>
> > In both classes I have overloaded an operator to facilitate addition.
>
> > ---------------------------------------------------------------------------­-------
> > /*Addition Operator Matrix plus Matrix*/
>
> > friend Matrix operator +(const Matrix& A, const Matrix& B)
> > {
>
> >     Matrix C(A.row, A.col);
> >     assert((A.row == B.row) && (A.col == B.col));
>
> I would rewrite it slightly.   Assert first, only then allocate.  Don't
> allocate the default, copy the 'A' matrix there.  Once you copied, use
> the compound assignment instead of regular one.
>
> Do
>
>      Matrix C(A); // copy-initialisation
>
> instead
>
>
>
> >         for(int m = 0; m<C.row; m++)
> >         {
> >             for(int n = 0; n<C.col; n++)
> >             {
> >                 C.data[m*C.col + n] = A.data[m*A.col + n] +
> > B.data[m*B.col + n];
>
> Do
>
>                    C.data[..] += B.data[..];
>
> instead.
>
> >             }
> >         }
>
> >     return C;
> > }
>
> If you created all necessary pieces to handle copying, you should have
> no problem.
>
>
>
>
>
>
>
> > ---------------------------------------------------------------------------­-------
> >  /*Addition Operator System plus System*/
>
> >         friend System operator +(const System& A, const System& B)
> >         {
>
> >             System C(A.num_eq);
> >             assert(A.num_eq == B.num_eq);
> >             for(int i = 0; i<C.num_eq; i++)
> >             {
> >                 C.equations[i] = A.equations[i] + B.equations[i];
> >             }
> >             return C;
> >         }
>
> > I have written a simple driver program to test the functionality of
> > these two classes. Given the following variables which have been
> > properly initialized
>
> > Matrix A, B, C, D;
>
> There is no default constructor in your Matrix class.  Hence I have a
> problem with this code of yours.  It's not the code you have.  See the
> FAQ 5.8.
>
> > System W, X, Y, Z;
>
> > D = A + B + C;         //produces valid results
> > Z = W + X;               //produces valid results
> > Z = W + X + Y;         //produces a segmentation fault
>
> > I have tried enclosing the terms within parenthesis but that does not
> > do anything.  The code for the operator+ is nearly identical for both
> > classes.  What causes the segmentation fault and how can I resolve it?
>
> > Justin
>
> > PS How do I inset code and preserve formatting?
>
> Make sure you have spaces instead of tabs.  If you have, I'm not sure
> what problem you are experiencing.
>
> All in all, I would say RTFFAQ first, and then ask other questions.
>
> V
> --
> Please remove capital 'A's when replying by e-mail
> I do not respond to top-posted replies, please don't ask- Hide quoted text -
>
> - Show quoted text -

I reposted my code into something that is compilable. I have been
using g++ ver 4.1.2 2007112. I have been careful to follow the rule
of the big three when making custom structures. I still get a
segmentation fault whenever I add three objects of type system
together.
I can add two objects of type System together withou any prolem.. i
/*-------Matrix
Class---------------------------------------------------------------------
*/

#include <assert.h>
#include <iostream>
using namespace std;
class Matrix
{

private:
int row; //number of rows;
int col; //number of col;
double *data; //pointer to pointers (2D data structure of the
matrix;)


public:
/*Constructor*/
Matrix():row(0), col(0){}

/*Constructor*/
Matrix(const int& M, const int& N): row(M), col(N)
{
data = new double[row*col]; //allocate memory for array of
type double
for(int m = 0; m<row; m++)
{
for(int n = 0; n<col; n++)
{
data[m*row + n] = 0;
}
}
}

/*Constructor for Initialized to values as specified in array A*/
Matrix(const double* A, const int& M, const int& N): row(M),
col(N)
{
data = new double[row*col]; //allocate memory for array
of type double
for(int m = 0; m<row; m++)
{
for(int n = 0; n<col; n++)
{
data[m*row + n] = A[m*row + n];
}
}
}

/*Copy Constructor*/
Matrix(const Matrix& old_Matrix): row(old_Matrix.row),
col(old_Matrix.col)
{
data = new double[row*col];
for(int m = 0; m<row; m++)
{
for(int n = 0; n<col; n++)
{
data[m*row + n] = old_Matrix.data[m*row + n];
}
}
}

/*Destructor*/
~Matrix()
{
delete[] data;
data = NULL;
}
/*Addition Operator Matrix plus Matrix*/

friend Matrix operator +(const Matrix& A, const Matrix& B)
{
assert((A.row == B.row) && (A.col == B.col));
Matrix C(A.row, A.col);
for(int m = 0; m<C.row; m++)
{
for(int n = 0; n<C.col; n++)
{
C.data[m*C.row + n] = A.data[m*A.row + n] + B.data[m*B.row
+ n];
}
}
return C;
}

/*-----System
Class----------------------------------------------------------------------
*/

#include "matrix.cpp"
using namespace std;
class System
{
private:
int num_eq; //number of differential equations in system
Matrix *equations; //array of class Matrix
public:
/*Constructor*/
System(const unsigned int num):num_eq(num)
{
equations = new Matrix[num_eq];
}
/*Copy Constructor*/
System(const System& old_System): num_eq(old_System.num_eq)
{
equations = new Matrix[num_eq];
for(int i = 0; i<num_eq; i++)
{
equations[i] = old_System.equations[i];
}
}
/*Destructor*/
~System()
{
delete[] equations;
equations = NULL;
}
/*Assignment Operator*/
const System& operator=(const System& rhs)
{
if (this != &rhs)
{
delete[] this->equations; // donate back useless memory
this->equations = new Matrix[rhs.num_eq]; // allocate new
memory
num_eq = rhs.num_eq;

for(int i = 0; i<num_eq; i++)
{
equations[i] = rhs.equations[i];
}
}
return *this; // return self-reference so cascaded assignment
works
}
/*Addition Operator System plus System*/

friend System operator +(const System& A, const System& B)
{
System C(A.num_eq);
assert(A.num_eq == B.num_eq);
for(int i = 0; i<C.num_eq; i++)
{
C.equations[i] = A.equations[i] + B.equations[i];
}
return C;
}

/*Accessor Functions*/

Matrix operator()(const unsigned short int a) const
{
assert(a < this->num_eq);
return this->equations[a];
}
Matrix& operator()(const unsigned short int a)
{
assert(a < this->num_eq);
return this->equations[a];
}
};


/*Ouput Operator*/
friend ostream & operator<<(ostream & out, const Matrix & A)
{
for(int m = 0; m<A.row; m++)
{
for(int n = 0; n<A.col; n++)
{
out << A.data[m*A.row + n]<<' ';
}
out <<endl;
}
return out;
}

/*Assignment Operator*/
const Matrix& operator=(const Matrix& rhs)
{
if (this != &rhs)
{
delete[] this->data; // donate back useless memory
this->data = new double[rhs.row*rhs.col]; // allocate new
memory
row = rhs.row;
col = rhs.col;
for(int m = 0; m<row; m++)
{
for(int n = 0; n<col; n++)
{
data[m*row + n] = rhs.data[m*rhs.row + n];
}
}
}
return *this; // return self-reference so cascaded assignment
works
}

double operator()(const unsigned short int a , const unsigned short
int b) const
{
assert((a < this->row) || (b < this->col));
return this->data[(a)*this->col + (b)];
}
double& operator()(const unsigned short int a , const unsigned
short int b)
{
assert((a < this->row) || (b < this->col));
return this->data[(a)*this->col + (b)];
}

};


/*------------driver
program-------------------------------------------------------*/

#include <cstdlib>
#include "system.cpp"
#define M 3 //number of rows
#define N 3 //number of col
using namespace std;


int main()
{
double a[9] = {1, 2, 3, 4, 5, 6, 7, 8, 9};
double b[9] = {1, 1, 1, 1, 1, 1, 1, 1, 1};
double c[9] = {1, 0, 1, 0, 1, 0, 1, 0, 1};
Matrix Er(a,M,N);
Matrix Ei(b,M,N);
Matrix G(c,M,N);

System Y(3);
System X(3);
Y(0) = Er;
Y(1) = Ei;
Y(2) = G;
cout<<Y(0)<<endl<<Y(1)<<endl<<Y(2)<<endl;

X = Y + Y;
cout<<X(0)<<endl<<X(1)<<endl<<X(2)<<endl;
X = Y + Y + Y;
cout<<X(0)<<endl<<X(1)<<endl<<X(2)<<endl;


return 0;
}

Any help would be appreciated.

Justin

Victor Bazarov

11/5/2008 11:07:00 PM

0

jr.freester@gmail.com wrote:
> I wrote
>> [..]
>> All in all, I would say RTFFAQ first, and then ask other questions.
>>
>> V
>> --
>> Please remove capital 'A's when replying by e-mail
>> I do not respond to top-posted replies, please don't ask- Hide quoted text -
>>
>> - Show quoted text -
>
> I reposted my code into something that is compilable.

No, you didn't. No matter, after some editing and changing 'private' to
'public', it compiled. And crashed. The error isn't obvious. See
below. And learn to use debuggers, they are your friends.

> I have been
> using g++ ver 4.1.2 2007112. I have been careful to follow the rule
> of the big three when making custom structures. I still get a
> segmentation fault whenever I add three objects of type system
> together.
> I can add two objects of type System together withou any prolem.. i
> /*-------Matrix
> Class---------------------------------------------------------------------
> */
>
> #include <assert.h>
> #include <iostream>
> using namespace std;
> class Matrix
> {
>
> private:
> int row; //number of rows;
> int col; //number of col;
> double *data; //pointer to pointers (2D data structure of the
> matrix;)
>
>
> public:
> /*Constructor*/
> Matrix():row(0), col(0){}

You initialise 'row', you initialise 'col'. What's 'data'? Since you
didn't initialise 'data', it contains *garbage*. Then you pass it to
the 'delete[]', KABOOM!

When I initialise 'data' to 0, the program completes fine and prints
some stuff

> ...
>
> Any help would be appreciated.
>
> Justin
>


--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask