Design issue in C#

[ddanbe]

I’m working on a little matrix class for my own use and because for me, it is just fun!
Now I noticed a lot (10 and more) of the methods always seem to follow the same pattern:
- Iterate over the rows
- For each row iterate over the columns
- Do something with each element of the matrix
An example:

// <returns>the zero matrix</returns>
        public static MMatrix Zero(int row, int col) 
        {
            MMatrix MA = new MMatrix(r, c);
            for (int i = 0; i < row; i++)
            {
                for (int j = 0; j < col; j++)
                {
                    MA[i, j] = 0;
                }
            }
            return MA;
        }

I asked myself if I could get rid of the nested for loops in all those methods.
So, I tried to improve on this, thanks to the wonders of C# by using an Action delegate:

public void Iterate(int rows, int cols, Action<int, int> A)
        {
            for (int i = 0; i < rows; i++)
            {
                for (int j = 0; j < cols; j++)
                {
                    A.Invoke(i,j);
                }
            }
        }

Now my Zero matrix method becomes:

public static ReMatrix Zero(int r, int c)
        {
            ReMatrix MA = new ReMatrix(r, c);
            MA.Iterate(r, c, (x, y) => { MA[x, y] = 0; });
            return MA;          
        }

I would like to ask you all, if this is an improvement?
Or is it better to stay with the nested for loops.

[deceptikon]

I'm a little biased as this kind of thing is something I use liberally to avoid duplication, but it's also a good practice in general.

One thing to note though is performance. Refactoring into methods and lambdas does tend to introduce overhead, and you should take that into consideration for the possibility of larger matrices.

[ddanbe]

Thanks for the reply.
I'm aware that it will probably generate a few machine cycles more. But it is not directly my intention to use matrices greater than, say 10X10.
So whenever possible and performance is not an issue you would advise this sort of programming practice?

[pritaeas]

Since you are refactoring, why are you passing the rows and cols to your iteration method? If you always iterate over all rows and columns, you can use the matrix internal rows and columns values instead (you passed them to the ctor already).

[deceptikon]

So whenever possible and performance is not an issue you would advise this sort of programming practice?

I wouldn't say whenever possible. Clarity trumps, so if duplicating code makes it easier to understand, that would be my recommendation. And if eliminating duplication is excessively confusing for one reason or another, I'd refrain. But in my experience those situations are somewhat rare if the code is otherwise well designed.

[overwraith]

I think it's an improvement, even with the slight overhead introduced. This could make other methods like an RREF function easier to code (it's been a while, I actually don't remember how to do row reduced echilon form, so I don't know). You could probably make this a little more interesting with addition and subtraction methods, RREF, as well as making it generic, and using dynamic casting where the generic inherits from struct to implement the addition and subtraction on the contents. You are talking about a mathematics matrix right?

T[] arr = new T[length];

arr[i] = (dynamic)arr[i] + (dynamic)otherarr[i];

Here is some code I wrote for a matrice program;

/*Author: overwraith*/
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Matrices {

    class Program {

        static void Main(string[] args) {

            Matrix<int> m1 = new Matrix<int>(new int[,] { { 0, 0, 1 }, { 0, 1, 1 } });
            Matrix<int> m2 = new Matrix<int>(new int[,] { { 0, 0, 1 }, { 0, 1, 1 } });
            Console.WriteLine(m1 + m2);


            Matrix<int> m3 = new Matrix<int>(new int[,] { { 0, 0, 1 } });
            Matrix<int> m4 = new Matrix<int>(new int[,] { { 0, 0, 1 } });
            Console.WriteLine(m3 + m4);
            Matrix<int> m = m3 + m4;
            m.To1DArray();

            Matrix<int> m5 = new Matrix<int>(new int[,] { { 0, 0, 1 }, { 0, 1, 1 }, { 1, 1, 1 } });
            Matrix<int> m6 = new Matrix<int>(new int[,] { { 0, 0, 1 }, { 0, 1, 1 }, { 1, 1, 1 } });
            Console.WriteLine(m5 + m6);

            pause();
        }//end main

        public static void pause() {
            Console.Write("Press any key to continue... ");
            Console.ReadLine();
        }

    }//end class

    public class Matrix<T> where T : struct {//basically a wrapper for an array
        //based on the algebra matrices
        T[,] matrix;

        //getters and setters
        public T[,] Arr {
            get {
                return matrix;
            }
        }

        //constructors
        public Matrix() { }

        public Matrix(T[,] input) {
            this.matrix = input;
        }

        //operators
        public static Matrix<T> operator +(Matrix<T> m1, Matrix<T> m2) {

            T[,] m1_arr = m1.Arr;
            T[,] m2_arr = m2.Arr;

            if(m1_arr.GetLength(0) != m2_arr.GetLength(0))
                throw new ArgumentException("Matrix dimensions do not match. ");

            if(m1_arr.GetLength(1) != m2_arr.GetLength(1))
                throw new ArgumentException("Matrix dimensions do not match. ");

            T[,] array = new T[m1_arr.GetLength(0), m1_arr.GetLength(1)];

            for (int i = 0; i < m1_arr.GetLength(0); i++)
                for (int j = 0; j < m2_arr.GetLength(1); j++)
                    array[i, j] = (dynamic)m1_arr[i, j] + (dynamic)m2_arr[i, j];

            return new Matrix<T>(array);
        }

        public override string ToString() {
            String str = "";

            int rowLen = matrix.GetLength(0);
            int colLen = matrix.GetLength(1);

            for (int i = 0; i < rowLen; i++) {
                for (int j = 0; j < colLen; j++)
                    str += matrix[i, j] + (j < colLen - 1 ? ", " : "");

                str += '\n';
            }


            return str;
        }

        public T[] To1DArray() {
            T[] arr;

            int rowLen = matrix.GetLength(0);
            int colLen = matrix.GetLength(1);

            if (Arr.GetLength(0) == 1) {//row length is 1, is 1 dimensional
                arr = new T[Arr.GetLength(1)];

                for (int i = 0; i < rowLen; i++)
                    for (int j = 0; j < colLen; j++)
                        arr[j] = Arr[i, j];

                return (T[])arr;
            }
            else
                throw new ArgumentException("Does not contain a one dimensional array. ");
        }

    }//end class

}//end namespace

There's some things I got to change, for instance the String in the toString method should be a string builder, which gets covnerted to a string. some of the code is old, and some is newer, I have learned some since I wrote the original.

[DaveAmour]

Hi

Sorry I'm a bit late coming to this thread now but pretty much agree with everything that has been said.

Do use self describing names for variables and methods though. So Iterate is maybe not the best method name - I would rethink that one.

Also naming conventions for things like variable names and method names etc are not set in stone and there are many standards out there. For me though and for most programmers I work with we tend to stick to the same standards as the framework code. Since our code is interspersed with framework code this makes sense. It is also I find a pretty well used standard across the industry. How do I know this? I've had about 40 jobs in IT as I'm a contractor so its based on experience.

So for:

MMatrix MA = new MMatrix(r, c);

I would use lower case for ma and also would use a whole, self describing word.

If you have time the video below is full of fantasic advice from a talented programmer and teacher and is also very funny too. He does also include a section on naming variables, methods etc.

https://www.youtube.com/watch?v=_Zqhs1IhGx4

[ddanbe]

Thanks to the advice of priteaus and the naming advise of DaveAmour(nice video Dave) I now have the following "iterate" method:

public void ForeachElement(Action<int, int> A)
        {
            for (int i = 0; i < this.Rows; i++)
            {
                for (int j = 0; j < this.Cols; j++)
                {
                    A.Invoke(i,j);
                }
            }
        }

I wonder if the IL code produced in both cases would differ that much. Know too little of IL to find that out. And in my case it doesn't matter so much.
Anyway thaks to all the contributors. :)

[DaveAmour]

I still don't like ForEachElement, I would prefer something that describes what it does and definatley don't like "A" as an argument name!

[ddanbe]

You're absolutely right!
Suggestions are always welcome.
I don't use one letter variable names very often and I try to invent some sensible names, but I find it sometimes hard, like in this case.
What I don't like is something like DataGridViewEditingControlShowingEventHandler from the DataGridView class.