So I recently had a practical exam in which I was told to execute a function of a menu oriented matrix operation program using classes. The code I made was supposedly wrong as I hadn't used classes the way they have to be used. Can anybody help me rectify the error here. I'm a noob (less than 2 months into C++) so please pardon any nooby mistakes.
#include <iostream>
using namespace std;
class matrix3
{
int a[10][10],b[10][10],c[10][10],d[10][10],e[10][10],f[10][10],x,y,i,j;
public :
void values();
void transpose();
void sum();
void diff();
};
void matrix3::values()
{
cout << "Enter the rows "; cin >> x;
cout << "Enter the columns "; cin >> y;
cout << "Enter elements of first matrix\n\n";
for (i=1; i<=x; i++)
{
for ( j=1; j<=y; j++)
{
cin >> a[i][j];
}
}
cout << "Enter elements of second matrix\n\n";
for (i=1; i<=x; i++)
{
for (j=1; j<=y; j++)
{
cin >> c[i][j];
}
}
}
void matrix3::sum()
{
cout << "Sum of Matrices 1 and 2 is\n";
for (i=1; i<=x; i++)
{
for ( j=1; j<=y; j++)
{
e[i][j]=a[i][j]+c[i][j];
cout << e[i][j] << "";
}
cout << endl;
}
}
void matrix3::diff()
{
cout << "Difference of Matrices 1 and 2 (1-2) is\n";
for (i=1; i<=x; i++)
{
for ( j=1; j<=y; j++)
{
f[i][j] = a[i][j]-c[i][j];
cout << f[i][j] << "";
}
cout << endl;
}
}
void matrix3::transpose()
{
cout << "transpose of the matrix is\n";
for ( i=1; i<=x; i++)
{
for ( j=1; j<=y; j++)
{
b[i][j] = a[j][i];
cout << b[i][j] << "";
}
cout << endl;
}
cout << "Transpose of the second matrix is\n";
for ( i=1; i<=x; i++)
{
for ( j=1; j<=y; j++)
{
d[i][j] = c[j][i];
cout << b[i][j] << "";
}
cout << endl;
}
}
int main()
{
int input;
char ch;
matrix3 m;
m.values();
do
{
cout << "Enter your choice\n";
cout << " 1. Sum of 1 and 2\n" << " 2. Difference of 1 and 2\n" << " 3. Transpose of both 1 amd 2\n";
cin >> input;
switch (input)
{
case 1:
m.sum();
break;
case 2:
m.diff();
break;
case 3:
m.transpose();
break;
}
cout << "\nDo another y/n?";
cin >> ch;
}
while (ch!= 'n');
cout << "\n";
system ("pause");
return 0;
}
4 Answers 4
Partial Answer This does not cover all aspects of your code, so feel free to wait for other responses.
In OOP, it is important to identify the scope of an object. In your case, you have two input matrices and possibly a result matrix for the operation that the user selects. The key point is, that each matrix should be a distinguished instance of your matrix
class. What you have is something like a matrix
manager, that contains multiple matrices in a single instance.
When you re-design your matrix
class to contain a single matrix, you should possibly externalize your user input logic. Also, the sum
, diff
and transpose
functions could be external functions that take two matrices as parameters and return a resulting matrix. However, I guess it would also be ok to have them as member function that only takes the second matrix as parameter.
Consider the following class definition:
#include <vector>
class matrix {
private:
// matrix with m columns and n rows
int _m, _n;
// values, organized as an outer vector of rows
// and an inner vector of columns
std::vector<std::vector<int>> _values;
public:
// initialize matrix by its size
matrix(int m, int n)
: _m(m), _n(n)
{
// initialize the matrix size
_values.resize(n);
for (int i = 0; i < n; ++i)
{
_values[i].resize(m);
}
}
// TODO:
// - other constructors
// - copy operator
// - other desired functions
// - assessors to the matrix elements
};
Maybe you can start from there.
-
\$\begingroup\$ the exact thing my teacher told me to do. can anyone tell me how to re-design it so as to contain a single matrix 'class' and not multiple matrices in a single instance? help appreciated :) \$\endgroup\$user2704772– user27047722015年11月27日 13:30:58 +00:00Commented Nov 27, 2015 at 13:30
-
\$\begingroup\$ @user2704772 Edited my answer to give you something to start with. \$\endgroup\$grek40– grek402015年11月27日 13:44:51 +00:00Commented Nov 27, 2015 at 13:44
-
\$\begingroup\$ umm. ill have to have a look at vectors (haven't studied it) to be able to get this. anyways thanks a lot. \$\endgroup\$user2704772– user27047722015年11月27日 13:48:57 +00:00Commented Nov 27, 2015 at 13:48
-
\$\begingroup\$ @user2704772 you can also replace the vector with your
int _values[10][10]
array, if you are sure that you never want any dimension size exceeding this value. I chose vector for its flexibility. \$\endgroup\$grek40– grek402015年11月27日 14:00:54 +00:00Commented Nov 27, 2015 at 14:00 -
\$\begingroup\$ Why would you store
_m
and_n
as members? \$\endgroup\$Loki Astari– Loki Astari2015年11月27日 18:02:11 +00:00Commented Nov 27, 2015 at 18:02
Your code seems to be wrong for few reasons.
First is from the basics of matrices. They are 0 index based. Meaning that in your loops you have to start from 0, not from 1. So if you declare a matrix a[10][10] the the indexes will be from 0 to 9.Following this, you will have:
for (i=0; i<x; i++) //since x is 10 you want to stop at the last index which is 9
If you only print the matrix operations results there is no need to save them somewhere, you can just print the results.Therefore you do not need to have the other matrices, two are enough. For example your sum function will be like this :
void matrix3::sum()
{
cout << "Sum of Matrices 1 and 2 is\n";
for (i=0; i<=x; i++)
{
for ( j=0; j<=y; j++)
{
cout << a[i][j]+c[i][j] << "";
}
cout << endl;
}
}
Also, you do not have a constructor ( which is the method that initializes your object) . This can be your values function since it assigns values to your matrices.
matrix3
is a user defined data type and C++ gives you a lot of freedom to make it the way you want. However, if you make it overly complicated people cannot use it effectively. As a rule of thumb you should aim to make your data types behave roughly like an int
because screwing up the usage of an int
is pretty hard.
To add 2 int
s you write something like this:
int a = 1;
int b = 2;
int c = a + b;
Since we are aiming to mimic int
s, the way we want to add matrices is this:
matrix3 a = ...;
matrix3 b = ...;
matrix3 c = a + b;
To print a matrix we would simply do this: std::cout << c << '\n'
.
What you did is combine a container (holding multiple objects) together with input/output and the actual functionality of matrix operations. You should try to make each class only have a single purpose. The only purpose that matrix3
should have is to do matrix operations.
Instead of writing an example by hand I'll refer you to eigen where they did all of this plus some other neat tricks.
Now imagine I did int c = a + b;
and my output said "The sum of 1 and 2 is 3". That would suck. Your matrix3
shouldn't do that either.
If you want to define Matrix
Operations in a class like way. You need to look at a matrix as an object (not a thing that holds three matrixes).
Matrix as an object.
What are the properties of a matrix?
1) Width
2) Height
3) Values (Data)
What are the action that can be performed on a matrix.
1) Sum
2) Diff
3) Transpose
Looking at these properties I would define the following.
class Matrix
{
std::size_t width;
std::size_t height;
std::vector<int> data;
public:
Matrix sum(Matrix const& rhs);
Matrix diff(Matrix const& rhs);
Matrix transpose();
};
You also need to define a couple of standard functions (or disable some standard functions).
class Matrix
{
public:
// Create a matrix (of specified width/height with all data set to zero
Matrix(int width, int height);
// Copy a matrix
Matrix(Matrix const& copy);
// Allow people to set matrixes via assignment
Matrix operator=(Matrix const& copy);
// Input and output operations so you can print a matrix
// And read a matrix from a stream (stdin/stdout)
friend std::ostream operator<<(std::ostream& str, Matrix const& output);
friend std::istream operator>>(std::istream& str, Matrix& input);
};
Once you have defined these you should be all set.
-
1\$\begingroup\$ Going the way of a 1d array, you may want to add a short section on translating the 2d coordinates to 1d index position. Other than that, it's a nice wrap-up of the things I didn't elaborate in my answer. \$\endgroup\$grek40– grek402015年11月27日 20:28:55 +00:00Commented Nov 27, 2015 at 20:28