8
\$\begingroup\$

Is there a more clever way of writing this piece of code? It does work but I thought there might be a better way of returning a matrix from multiplying the parameter by itself.

public static void Main(string[] args)
{
    int[,] m1 = CreateMatrix(5);            
}

static int[,] CreateMatrix(int rowscols)
{            
    int[,] result = new int[rowscols, rowscols];

    int res = rowscols * rowscols;
    int counter= 0;

    for (int i = 0; i < rowscols; ++i)
    {                                                
        for (int k = 0; k < rowscols; ++k)
        {                    
            result[i, k] = (res - counter);
            counter += 1;
        }                
    }

    return result;
}
\$\endgroup\$
5
\$\begingroup\$

rowscols is not a very good name. Although on one hand it's "accurate", on another it's kind of awkward. How about dimension or dim instead?

res is also not a very good name. Since its value is dim * dim, it's the number of cells in the matrix, so cellCount would be better.

It would be better to move the counter variable inside the loop, by declaring it in the outer loop together with i, and incrementing it in the innermost loop together with k.

It's a common practice to name loop variables i, j, k. You skipped j and went directly for k. It's not a "problem", but j would be more natural, especially because at school (I think) they teach matrices typically with \$m_{ij}\$ notation rather than \$m_{ik}\$

Lastly, since the result of the function is a matrix, it will be more meaningful to call the variable matrix instead of result.

Suggested implementation

Putting the above suggestions together:

static int[,] CreateMatrix(int dimension)
{            
    var matrix = new int[dimension, dimension];

    var cellCount = dimension * dimension;

    for (int i = 0, counter = 0; i < dimension; ++i)
    {                                                
        for (int j = 0; j < dimension; ++j, ++counter)
        {                    
            matrix[i, j] = cellCount - counter;
        }                
    }

    return matrix;
}

Special thanks to @Abbas for pointing out to use the var keyword instead of explicitly declaring the matrix and cellCount variables.

\$\endgroup\$
  • 1
    \$\begingroup\$ +1, very nice answer. I'd only like to add two things. 1. Use the var keyword instead of explicitly declaring your variables. dim is as meaningful as rowscols. Ok, a but better but still... I would strongly prefer dimension. \$\endgroup\$ – Abbas Oct 30 '14 at 20:52
  • 1
    \$\begingroup\$ Thanks @Abbas, I agree with your points and updated my answer accordingly. \$\endgroup\$ – janos Oct 30 '14 at 22:09

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.