patterncsharpMinor
Immutable Matrix
Viewed 0 times
immutablematrixstackoverflow
Problem
I'm writing implementations of some numerical methods to solve linear equations systems, those implementations use the following Matrix class. I'm trying to get this class immutable, due to that the constructor and some methods create a copies of the backing two-dimesional array. The class have some basic functionality to operate NxM Matrix e.g. overloaded operators for sum, difference and product. The methods Equals(object) and HashCode() were stubbed by Resharper.
I'd would like to read any suggestions and improvements.
```
public class Matrix
{
public int M { get; private set; }
public int N { get; private set; }
private readonly double[,] _values;
public Matrix(double[,] values)
{
M = values.GetLength(0);
N = values.GetLength(1);
_values = (double[,])values.Clone();
}
public double this[int row, int column] {
get
{
ValidateRowIndex(row);
ValidateColumnIndex(column);
return _values[row, column];
}
}
public IEnumerable Column(int column)
{
ValidateColumnIndex(column);
for (int i = 0; i Row(int row)
{
ValidateRowIndex(row);
for (int j = 0; j
/// Method that will add a row times a factor to other row
/// in the system.
///
/// Row to be added
/// Row that will be added to
/// Times the row1 will be multiplied before be added
/// Result matrix
public Matrix AddRow(int row1, int row2, double factor)
{
ValidateRowIndex(row1);
ValidateRowIndex(row2);
if (factor == 0)
{
throw new ArgumentException("factor cannot be zero");
}
var result = new double[M, N];
var row = Row(row1).ToArray();
for (int i = 0; i n1 + n2);
}
public static Matrix operator -(Matrix a, Matrix b)
{
ValidateSameDimensions(a,
I'd would like to read any suggestions and improvements.
```
public class Matrix
{
public int M { get; private set; }
public int N { get; private set; }
private readonly double[,] _values;
public Matrix(double[,] values)
{
M = values.GetLength(0);
N = values.GetLength(1);
_values = (double[,])values.Clone();
}
public double this[int row, int column] {
get
{
ValidateRowIndex(row);
ValidateColumnIndex(column);
return _values[row, column];
}
}
public IEnumerable Column(int column)
{
ValidateColumnIndex(column);
for (int i = 0; i Row(int row)
{
ValidateRowIndex(row);
for (int j = 0; j
/// Method that will add a row times a factor to other row
/// in the system.
///
/// Row to be added
/// Row that will be added to
/// Times the row1 will be multiplied before be added
/// Result matrix
public Matrix AddRow(int row1, int row2, double factor)
{
ValidateRowIndex(row1);
ValidateRowIndex(row2);
if (factor == 0)
{
throw new ArgumentException("factor cannot be zero");
}
var result = new double[M, N];
var row = Row(row1).ToArray();
for (int i = 0; i n1 + n2);
}
public static Matrix operator -(Matrix a, Matrix b)
{
ValidateSameDimensions(a,
Solution
It would be better to do the faster checks first in this method:
So we have this:
There's no reason for
On comparing doubles, I'll quote from MSDN:
The
equivalent values can be unequal due to the differing precision of the
two values. The following example reports that the
.333333 and the
...
Rather than comparing for equality, one technique involves defining an
acceptable relative margin of difference between two values (such as
.001% of one of the values). If the absolute value of the difference
between the two values is less than or equal to that margin, the
difference is likely to be due to differences in precision and,
therefore, the values are likely to be equal.
We can write this in a more efficient way:
We should also be checking that
protected bool Equals(Matrix other)
{
return _values.Cast()
.SequenceEqual(other._values.Cast())
&& M == other.M && N == other.N;
}So we have this:
protected bool Equals(Matrix other)
{
return M == other.M &&
N == other.N &&
_values.Cast().SequenceEqual(other._values.Cast());
}Cast.SequenceEqual is nice and short, but there might be performance issues (you would need to test this). I think the more verbose option would be better in terms of clarity (and possibly performance):if (M != other.M || N != other.N)
{
return false;
}
for (int i = 0; i < M; i++)
{
for (int j = 0; j < N; j++)
{
if (_values[i, j] != other._values[i, j])
{
return false;
}
}
}
return true;There's no reason for
Equals(Matrix) to be protected; in fact, if we make it public we can let Matrix implement IEquatable (not forgetting to add a null check).On comparing doubles, I'll quote from MSDN:
The
Equals method should be used with caution, because two apparentlyequivalent values can be unequal due to the differing precision of the
two values. The following example reports that the
Double value.333333 and the
Double value returned by dividing 1 by 3 are unequal....
Rather than comparing for equality, one technique involves defining an
acceptable relative margin of difference between two values (such as
.001% of one of the values). If the absolute value of the difference
between the two values is less than or equal to that margin, the
difference is likely to be due to differences in precision and,
therefore, the values are likely to be equal.
public static Matrix GetIdentityMatrix(int order)
{
var identity = new double[order, order];
for (int i = 0; i < order; i++)
{
for (int j = 0; j < order; j++)
{
if (i == j)
{
identity[i, j] = 1;
}
}
}
return identity;
}We can write this in a more efficient way:
public static Matrix GetIdentityMatrix(int order)
{
var identity = new double[order, order];
for (int i = 0; i < order; i++)
{
identity[i, i] = 1;
}
return identity;
}We should also be checking that
order is positive.Code Snippets
protected bool Equals(Matrix other)
{
return _values.Cast<double>()
.SequenceEqual(other._values.Cast<double>())
&& M == other.M && N == other.N;
}protected bool Equals(Matrix other)
{
return M == other.M &&
N == other.N &&
_values.Cast<double>().SequenceEqual(other._values.Cast<double>());
}if (M != other.M || N != other.N)
{
return false;
}
for (int i = 0; i < M; i++)
{
for (int j = 0; j < N; j++)
{
if (_values[i, j] != other._values[i, j])
{
return false;
}
}
}
return true;public static Matrix GetIdentityMatrix(int order)
{
var identity = new double[order, order];
for (int i = 0; i < order; i++)
{
for (int j = 0; j < order; j++)
{
if (i == j)
{
identity[i, j] = 1;
}
}
}
return identity;
}public static Matrix GetIdentityMatrix(int order)
{
var identity = new double[order, order];
for (int i = 0; i < order; i++)
{
identity[i, i] = 1;
}
return identity;
}Context
StackExchange Code Review Q#64474, answer score: 6
Revisions (0)
No revisions yet.