patterncsharpMinor
2D matrices addition
Viewed 0 times
additionmatricesstackoverflow
Problem
I have a 2 dimension matrix :
And a method to sum them :
At the moment, the complexity is of \$O(mnp)\$, \$m\$ being the number of matrices and \$n\$ being the row length of matrices and \$p\$ the column length.
I'm wondering if there are edge cases that might crash my method, is it a good decision to make my
public struct Matrix2D
{
private double[,] matrix;
public Matrix2D(double[,] matrix)
{
if (matrix == null) throw new ArgumentNullException(nameof(matrix));
this.matrix = matrix;
}
private double[,] Matrix => matrix ?? (matrix = new double[0, 0]);
public int RowLength => Matrix.GetLength(1);
public int ColumnLength => Matrix.GetLength(0);
public double this[int row, int column] => Matrix[row, column];
public double[,] AsArray() => (double[,])Matrix.Clone();
}And a method to sum them :
///
/// Sums multiple matrices togheter
///
/// Matrices to add
/// New matrix with the summated values
public static Matrix2D Sum(params Matrix2D[] matrices)
{
if (matrices == null) throw new ArgumentNullException(nameof(matrices));
if (matrices.Length == 0) return new Matrix2D();
if (matrices.Length == 1) return matrices[0];
int rowLength = matrices[0].RowLength;
int columnLength = matrices[0].ColumnLength;
double[,] matrix = matrices[0].AsArray();
for (int matrixIndex = 1; matrixIndex < matrices.Length; matrixIndex++)
{
var matrix2D = matrices[matrixIndex];
if (matrix2D.RowLength != rowLength || matrix2D.ColumnLength != columnLength)
throw new ArgumentException("Matrixes must have the same size");
for (int row = 0; row < rowLength; row++)
{
for (int column = 0; row < columnLength; row++)
{
matrix[row, column] += matrix2D[row, column];
}
}
}
return new Matrix2D(matrix);
}At the moment, the complexity is of \$O(mnp)\$, \$m\$ being the number of matrices and \$n\$ being the row length of matrices and \$p\$ the column length.
I'm wondering if there are edge cases that might crash my method, is it a good decision to make my
Matrix2D a struct and is there a way to make this more performant? Obviously any other comments are welcoSolution
A couple of changes I would suggest.
To Struct or not to Struct:
Structs are useful for value equality situations. This is objects which require constant value equality checks or intend to be used as keys in Dictionaries. However what I see in use is more of an immutable class. While structs are considered to be best practice as immutable objects you can do the same with classes, but you get to sidestep the inevitable equality issue.
You see, if you were to use equality on your current Matrix object you will find it is slow to check. In fact structs can even end up using reflection to check equality in the worst case, which is seriously inefficient. see C# - Value Type Equals method - why does the compiler use reflection?
Because of this you should always overload equality when using structs, if only for speed reasons. See What needs to be overriden in a struct to ensure equality operates properly? or one of the many other articles online about this.
Your use case doesn't seem to use or need equality, therefore I would suggest a class over a struct. As a general rule if you don't see yourself using the object as a dictionary key or for equality I wouldn't use a struct.
Var:
On a less important note I am always a strong advocate of using var in your code to make refactoring easier. In the following lines you are declaring the type twice:
There are actually quite a few more places on top of this. This seems trivial but the usage of var over say double[,] or int (for the case of row) means that if you have to refactor the class into a float[,] or to return a long for a size, that your current code would still function with no effort on your part. Var tends to save on time typing long types and is indispensable for refactoring in looping. It can be a bit debatable, as each team has there own standards, but I can tell you on my team we have an "all var all the time" rule which has been a godsend for refactoring.
{'s:
You swap between style of whether you use a trailing set of {'s between lines. For your for loop you use them, but your if statement does not. While I would advocate always using them, even if you only have a single line in the statement, I would say it's more important to be consistent. If you are going to use the style without {'s for single line statements, do so everywhere. If not, which I generally suggest, do not. This is more of a taste issue, and certainly not a law of the land, but worth noting.
Otherwise this looks good. Hope this helps.
To Struct or not to Struct:
Structs are useful for value equality situations. This is objects which require constant value equality checks or intend to be used as keys in Dictionaries. However what I see in use is more of an immutable class. While structs are considered to be best practice as immutable objects you can do the same with classes, but you get to sidestep the inevitable equality issue.
You see, if you were to use equality on your current Matrix object you will find it is slow to check. In fact structs can even end up using reflection to check equality in the worst case, which is seriously inefficient. see C# - Value Type Equals method - why does the compiler use reflection?
Because of this you should always overload equality when using structs, if only for speed reasons. See What needs to be overriden in a struct to ensure equality operates properly? or one of the many other articles online about this.
Your use case doesn't seem to use or need equality, therefore I would suggest a class over a struct. As a general rule if you don't see yourself using the object as a dictionary key or for equality I wouldn't use a struct.
Var:
On a less important note I am always a strong advocate of using var in your code to make refactoring easier. In the following lines you are declaring the type twice:
int rowLength = matrices[0].RowLength;
int columnLength = matrices[0].ColumnLength;
double[,] matrix = matrices[0].AsArray();
for (int row = 0; row < rowLength; row++){...}There are actually quite a few more places on top of this. This seems trivial but the usage of var over say double[,] or int (for the case of row) means that if you have to refactor the class into a float[,] or to return a long for a size, that your current code would still function with no effort on your part. Var tends to save on time typing long types and is indispensable for refactoring in looping. It can be a bit debatable, as each team has there own standards, but I can tell you on my team we have an "all var all the time" rule which has been a godsend for refactoring.
{'s:
You swap between style of whether you use a trailing set of {'s between lines. For your for loop you use them, but your if statement does not. While I would advocate always using them, even if you only have a single line in the statement, I would say it's more important to be consistent. If you are going to use the style without {'s for single line statements, do so everywhere. If not, which I generally suggest, do not. This is more of a taste issue, and certainly not a law of the land, but worth noting.
Otherwise this looks good. Hope this helps.
Code Snippets
int rowLength = matrices[0].RowLength;
int columnLength = matrices[0].ColumnLength;
double[,] matrix = matrices[0].AsArray();
for (int row = 0; row < rowLength; row++){...}Context
StackExchange Code Review Q#110700, answer score: 3
Revisions (0)
No revisions yet.