5
\$\begingroup\$

I just wanted to know if I'm doing this right. Note that all the matrices are int[16]s.

for (int i= 1; i <= 3; i++){
    for (int j= 1; j <= 3; j++){
        int sum = 0;
        for (int h = 1; h <= 3; h++){
           sum = sum + position[i*4+h]*projection[h*4+j];
        }

        modelview[i*4+j] = sum;
    }
}
\$\endgroup\$

migrated from stackoverflow.com Jan 3 '12 at 13:07

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ It looks like you are indexing a vector (one dimension) rather than a matrix (at least 2 dimensions), \$\endgroup\$ – JonnyBoats Jan 2 '12 at 5:03
  • \$\begingroup\$ Are you representing a 4 x 4 as a 1 x 16? Are you getting the right answer? \$\endgroup\$ – Joe McGrath Jan 2 '12 at 5:05
  • 2
    \$\begingroup\$ @JonnyBoats - no he is storing the 2d matrix in a 1d array, very common in c \$\endgroup\$ – Martin Beckett Jan 2 '12 at 5:05
  • \$\begingroup\$ Well, it's a one d array, yes, but it's a 16 size array, which is 4x4. You don't need a 2d array to have 2 dimensions in an array, just got to write code like [width * height] or [(width * 2) + 3] instead of [2][3]. \$\endgroup\$ – CyanPrime Jan 2 '12 at 5:06
  • 1
    \$\begingroup\$ Kinda nit picking here, but your title "multiplying two matrices together" would be better phrased "one matrix by another" or just "multiplying matrices" since order matters. \$\endgroup\$ – Joe McGrath Jan 2 '12 at 5:11
12
\$\begingroup\$

No. Let's first look at it as two dimensional arrays, so we know what we're talking about:

int i[4][4];
int j[4][4];
int k[4][4];

for (int x = 0; x < 4; x++) { // row number of output
    for (int y = 0; y < 4; y++) { // column number of output
        k[x][y] = 0;
        for (int z = 0; z < 4; z++) { // four elements are added for this output
            k[x][y] += i[x][z] * j[z][y];
        }
    }
}

Important difference!!! We start at 0 for the indices!

Now I'll convert it to int[16]s for you assuming that i[1] is the second element of the first row:

int i[16];
int j[16];
int k[16];

for (int x = 0; x < 4; x++) { // row number of output
    for (int y = 0; y < 4; y++) { // column number of output
        k[4*x+y] = 0;
        for (int z = 0; z < 4; z++) { // four elements are added for this output
            k[4*x+y] += i[4*x+z] * j[4*z+y];
        }
    }
}

This is equivalent to your code except for the fact that you start at 1 instead of zero.

\$\endgroup\$
1
\$\begingroup\$

@Dan "equivalent ... except .. you start at 1 instead of zero" solves the OP's first problem, but not the OP's second problem.

The OP's second and larger problem is functioning 3x3 matrix multiplier code that reads enough like buggy 4X4 matrix multiplier code to trip up its reviewers. I suspect the problems STARTED with the common mistake of using 1-based counting instead of 0-based indexing. And that was followed by a superficial solution: just pad the indexed vectors so that the 1-based counts are still in range, never mind the unused gaps.

The reviewer sees the idiomatic i*4+j and thinks "AHA! 4 MUST be the matrix dimension." implicitly reasoning "...because otherwise there would be unused gaps".

This miscue can be pinned on the use of "magic numbers", namely 3 and 4. Which of these is the intended matrix dimension? If the magic numbers had been referenced by name/purpose rather than by value, like via

    const unsigned int DIMENSION = 3;
    const unsigned int ROW_LENGTH = DIMENSION + 1;

the intent would have been clear and the mis-step of padding the vectors much easier to spot.

Or maybe I'm completely wrong and it always was just buggy 4x4 matrix multiplier code that coincidentally gave the right answer for 3x3 matrices.

Regardless, the common wisdom goes: Unless there is a very good reason to do otherwise,

  • use 0-based indexing, because it's simple, efficient, and idiomatic.

  • likewise, use the for loop < idiom for 0-based index iteration:

    for (unsigned int i = 0; i < DIMENSION; i++) {
    
  • reference special numbers in the problem domain via named constants, for self-documentation (as well as for flexibility).

\$\endgroup\$

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.