0
\$\begingroup\$

I would like to ask how I can make my code more simple and effective. I know that this code can be 100% better. I am supposed to load number for each array and then take the 2 arrays (4*3 and 3*4) and multiply them into 3*3 array.

The main point is making thescanf_s for both int a and int b in one "piece of code."

#include "stdafx.h"
#include <cstdlib>
#include <iostream>
#include <cmath>
using namespace std;

int main()
{
    int a[3][4];
    int b[4][3];
    int c[3][3] = { {0, 0, 0}, {0, 0, 0}, {0, 0, 0} };

    for (int j = 0; j != 4; j++)
    {
        for (int l = 0; l != 4; l++)
        {
            cout << "Zadej a[" << j << "," << l << "]: ";
            scanf_s("%d", &a[j][l]);
        }
    }

    for (int j = 0; j != 4; j++)
    {
        for (int l = 0; l != 4; l++)
        {
            cout << "Zadej b[" << j << "," << l << "]: ";
            scanf_s("%d", &b[j][l]);
        }
    }


    for (int j = 0; j != 3; j++)
    {
        for (int l = 0; l != 3; l++)
        {
            ;
            printf(" |%d| ", c[j][l] += a[j][l] * b[j][l]);
        }
        printf("\n");
    }

    return 0;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ "Is the code working the right way ?" It's you who's in charge to ensure that, before asking for a review here. \$\endgroup\$ – πάντα ῥεῖ Jan 16 at 17:56
  • \$\begingroup\$ Don't take me the wrong way, it is working but I have never done anything with matrixes yet so I am not sure if I'm counting it the right way. \$\endgroup\$ – Patrik Šoukal Jan 16 at 18:01
  • 4
    \$\begingroup\$ Write appropriate test cases first to the best of your knowledge. \$\endgroup\$ – πάντα ῥεῖ Jan 16 at 18:02
  • 1
    \$\begingroup\$ Your new code was barely an improvement compared to the old code. Please wait a while longer, more answers might be incoming. Feel free to take another good look at it yourself as well. Should you write a piece that's definitely improved, write a follow-up question linking back to this one for bonus context. \$\endgroup\$ – Mast Jan 16 at 19:50
  • 2
    \$\begingroup\$ ...Sorry, Wikipedia was bad advice, that is terribly obtusely written. Here is a better explanation. \$\endgroup\$ – Cris Luengo Jan 17 at 3:01
3
\$\begingroup\$

Don't put everything into one big main()

If you can separate out the reading of inputs and writing of results from the actual multiplications, then it will be easier to test the multiplication code separately.

Avoid non-standard libraries

Here we have "stdafx.h" and scanf_s that aren't part of standard C++. Ditch those and use the standard facilities (e.g. std::cin >> a[j][l]).

Include what you use

We don't seem to use <cmath> anywhere, so let's drop that. We'll need <cstdio> for std::printf() - or switch to C++ style output using <iostream>.

Avoid using the whole std namespace

The std namespace isn't one of the few that's designed to be imported wholesale like that, and there's potential for name conflicts when moving to a new standards version. Specify just the names you need, or just get used to writing std:: - it's intentionally very short.

Use C++ collections

It's easier to work with the C++ collection types such as std::array or std::vector than with raw (C-style) arrays (which decay to pointers when passed as function arguments).

\$\endgroup\$
  • \$\begingroup\$ So I edited the code a bit but now it posts an error: "no operator "<<" matches these operands". Also now that I removed the [] brackets how am I supposed to change the x/y the same way I did before? std::array<int, 3> a; std::array<int, 4> b; std::array<int, 3> c; \$\endgroup\$ – Patrik Šoukal Jan 16 at 19:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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