0
\$\begingroup\$

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;
 }
BCdotWEB
11.4k2 gold badges28 silver badges45 bronze badges
asked Nov 27, 2015 at 12:35
\$\endgroup\$

4 Answers 4

3
\$\begingroup\$

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.

answered Nov 27, 2015 at 13:22
\$\endgroup\$
6
  • \$\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\$ Commented Nov 27, 2015 at 13:30
  • \$\begingroup\$ @user2704772 Edited my answer to give you something to start with. \$\endgroup\$ Commented 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\$ Commented 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\$ Commented Nov 27, 2015 at 14:00
  • \$\begingroup\$ Why would you store _m and _n as members? \$\endgroup\$ Commented Nov 27, 2015 at 18:02
2
\$\begingroup\$

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.

answered Nov 27, 2015 at 13:15
\$\endgroup\$
1
\$\begingroup\$

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 ints you write something like this:

 int a = 1;
 int b = 2;
 int c = a + b;

Since we are aiming to mimic ints, 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.

answered Nov 27, 2015 at 13:31
\$\endgroup\$
1
\$\begingroup\$

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.

answered Nov 27, 2015 at 18:41
\$\endgroup\$
1
  • 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\$ Commented Nov 27, 2015 at 20:28

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.