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;
    }
\$\endgroup\$
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.

\$\endgroup\$
  • \$\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 Nov 27 '15 at 13:30
  • \$\begingroup\$ @user2704772 Edited my answer to give you something to start with. \$\endgroup\$ – grek40 Nov 27 '15 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 Nov 27 '15 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 Nov 27 '15 at 14:00
  • \$\begingroup\$ Why would you store _m and _n as members? \$\endgroup\$ – Martin York Nov 27 '15 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.

\$\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.

\$\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.

\$\endgroup\$
  • 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 Nov 27 '15 at 20:28

protected by Jamal Nov 28 '17 at 23:23

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.