Optimizing Matrix class with less calls to new

Question

I'm designing a Matrix class and I feel like what I've come up with requires too many calls to new. I know it's possible that caring about this is premature, but I already know I need this code to be fast, so I'd like to think about streamlining it now.

Here's a test to show how I intend to use the API:

[Fact]
public void TranslateFluentAPI()
{
 Matrix4 t = Matrix4.Identity().Translate(5, -3, 2);
 Point p = new Point(-3, 4, 5);
 Assert.True(t * p == new Point(2, 1, 7));
 // Same, but use the parameterless c'tor, which is equivalent to
 // calling Identity();
 Matrix4 t2 = new Matrix4().Translate(5, -3, 2);
 Assert.True(t2 * p == new Point(2, 1, 7));
}

And my code stripped down to the relevant parts (only missing additional matrix math and similar subclasses for Matrix3, Matrix2)

using System;
using static System.Math;
public abstract class Matrix : IEquatable<Matrix>
{
 // 2D array that stores Matrix data.
 protected double[,] data;
 // Concrete Matrix classes must override a size.
 abstract protected int size { get; }
 public Matrix(double[,] data)
 {
 if (data.GetLength(0) != size || data.GetLength(1) != size) {
 throw new ArgumentException(
 $"The provided 2D array needs to have a length of {size} "+
 "in both dimensions.");
 }
 this.data = data;
 }
 public Matrix()
 {
 // Do nothing.
 }
 // Square bracket getter/setter
 public double this[int x, int y] {
 get { return this.data[x, y]; }
 set { this.data[x, y] = value; }
 }
 #region Equality
 public bool Equals(Matrix other)
 {
 if (size != other.size) {
 throw new ArgumentException(
 $"Cannot compare a matrix of size {other.size} to a matrix"+
 $"of size {size}.");
 }
 double epsilon = 0.00001;
 for (int x = 0; x < size; x += 1) {
 for (int y = 0; y < size; y += 1) {
 if (! (Abs(this[x, y] - other[x, y]) < epsilon)) {
 // One element is not equal, so matrices are not equal.
 return false;
 }
 }
 }
 // All elements were equal
 return true;
 }
 public override bool Equals(object obj)
 {
 return Equals(obj as Matrix);
 }
 public override int GetHashCode()
 {
 return data.GetHashCode();
 }
 
 public static bool operator ==(Matrix obj1, Matrix obj2)
 {
 return obj1.Equals(obj2);
 }
 public static bool operator !=(Matrix obj1, Matrix obj2)
 {
 return ! obj1.Equals(obj2);
 }
 #endregion
}
public class Matrix4 : Matrix
{
 protected override int size => 4;
 public Matrix4(double[,] data) : base(data)
 {
 // Do nothing, calling base class.
 }
 public Matrix4() : base()
 {
 data = Matrix4.Identity().data;
 }
 public static implicit operator Matrix4(double[,] m) => new Matrix4(m);
 public static Matrix4 Identity()
 {
 return new Matrix4(new double[,] {
 {1, 0, 0, 0},
 {0, 1, 0, 0},
 {0, 0, 1, 0},
 {0, 0, 0, 1},
 });
 }
 public static Matrix4 operator *(Matrix4 a, Matrix4 b)
 {
 double[,] result = new double[4, 4];
 for (int x = 0; x < a.size; x += 1) {
 for (int y = 0; y < a.size; y += 1) {
 result[x, y] =
 a[x, 0] * b[0, y] +
 a[x, 1] * b[1, y] +
 a[x, 2] * b[2, y] +
 a[x, 3] * b[3, y];
 }
 }
 return new Matrix4(result);
 }
 public Matrix4 Translate(double x, double y, double z)
 {
 Matrix4 t = Matrix4.Identity();
 t[0, 3] = x;
 t[1, 3] = y;
 t[2, 3] = z;
 this.data = (t * this).data;
 return this;
 }
}

So adding up calls to new:

// Using the static Identity method:
Matrix4 t = Matrix4.Identity().Translate(5, -3, 2);
// This is 5 calls to new.
// As a convenience I have the parameterless c'tor to return the identity,
// so that you can do this:
Matrix4 t = new Matrix4().Translate(5, -3, 2);
// This is 6 calls to new.

Premature optimization aside, at the very least I wish I could make the second form not cost more than the first, because it feels like a much more intuitive way to use the API.

• ack: @Heslacher I'm not sure what you think you're missing. I assure you the implementation of IEquatable, ToString, and a few operators are not at all relevant this question about c'tors. I probably should have posted to SO but they usually downvote for the question being too broad ...
• Heslacher: Well at least the implementation of the * operator is needed.
• ack: @Heslacher ah sure thing, just updated. I believe that code is entirely self-contained now.
• Heslacher: Ok, for me it seems sufficient code now. If you don't receive a review I will check it out on monday.

Answer 1

First let's target your main concern regarding Matrix4 t = new Matrix4().Translate(5, -3, 2); using the same number of ctor calls as Matrix4 t = Matrix4.Identity().Translate(5, -3, 2);.

This can be easily achieved by extracting the double[,] out of Identity() like so

private static readonly double[,] identityData = new double[,] {
 {1, 0, 0, 0},
 {0, 1, 0, 0},
 {0, 0, 1, 0},
 {0, 0, 0, 1},
};
public static Matrix4 Identity()
{
 return new Matrix4(identityData);
} 

now the call to the parameterless ctor will look like so

public Matrix4() : base()
{
 data = identityData;
} 

---

In the Translate() method you are only interested in the data of the multiplied matrixes. You could extract the multiplication into a separate Multiply() method which would be called by the Translate() method and in the * operator like so

public static Matrix4 operator *(Matrix4 a, Matrix4 b)
{
 return new Matrix4(Multiply(a.data, b.data));
}
private static double[,] Multiply(double[,] a, double[,] b)
{
 int size = a.GetLength(0);
 double[,] result = new double[size, size];
 for (int x = 0; x < size; x += 1)
 {
 for (int y = 0; y < size; y += 1)
 {
 result[x, y] =
 a[x, 0] * b[0, y] +
 a[x, 1] * b[1, y] +
 a[x, 2] * b[2, y] +
 a[x, 3] * b[3, y];
 }
 }
 return result;
}
public Matrix4 Translate(double x, double y, double z)
{
 Matrix4 t = Matrix4.Identity();
 t[0, 3] = x;
 t[1, 3] = y;
 t[2, 3] = z;
 this.data = Multiply(t.data, this.data);
 return this;
} 

It is only a matter of taste, but I prefer to use e.g x++ over x +=1.

---

You could save one more call to the ctor by just using the identityData inside the Translate() method like so

public Matrix4 Translate(double x, double y, double z)
{
 double[,] t = identityData;
 t[0, 3] = x;
 t[1, 3] = y;
 t[2, 3] = z;
 this.data = Multiply(t, this.data);
 return this;
} 

Summing up will lead to

public class Matrix4 : Matrix
{
 protected override int size => 4;
 public Matrix4(double[,] data) : base(data)
 { } // Do nothing, calling base class.
 public Matrix4() : base()
 {
 data = identityData;
 }
 public static implicit operator Matrix4(double[,] m) => new Matrix4(m);
 private static readonly double[,] identityData = new double[,] {
 {1, 0, 0, 0},
 {0, 1, 0, 0},
 {0, 0, 1, 0},
 {0, 0, 0, 1},
 };
 public static Matrix4 Identity()
 {
 return new Matrix4(identityData);
 }
 public static Matrix4 operator *(Matrix4 a, Matrix4 b)
 {
 return new Matrix4(Multiply(a.data, b.data));
 }
 private static double[,] Multiply(double[,] a, double[,] b)
 {
 int size = a.GetLength(0);
 double[,] result = new double[size, size];
 for (int x = 0; x < size; x += 1)
 {
 for (int y = 0; y < size; y += 1)
 {
 result[x, y] =
 a[x, 0] * b[0, y] +
 a[x, 1] * b[1, y] +
 a[x, 2] * b[2, y] +
 a[x, 3] * b[3, y];
 }
 }
 return result;
 }
 public Matrix4 Translate(double x, double y, double z)
 {
 double[,] t = identityData;
 t[0, 3] = x;
 t[1, 3] = y;
 t[2, 3] = z;
 this.data = Multiply(t, this.data);
 return this;
 }
}

Now Matrix4 t = Matrix4.Identity().Translate(5, -3, 2); and Matrix4 t = new Matrix4().Translate(5, -3, 2); only call the ctor once. You need to decide if the code is still readable and maintainable.

---

Base class

The Equals(Matrix) should check for null for the passed Matrix other. Its better to return false if other == null than to get a NullReferenceException.

public bool Equals(Matrix other)
{
 if (other == null) { return false; }
 if (size != other.size)
 {
 throw new ArgumentException(
 $"Cannot compare a matrix of size {other.size} to a matrix" +
 $"of size {size}.");
 }
 double epsilon = 0.00001;
 for (int x = 0; x < size; x += 1)
 {
 for (int y = 0; y < size; y += 1)
 {
 if (!(Abs(this[x, y] - other[x, y]) < epsilon))
 {
 return false;
 }
 }
 }
 return true;
}

Both, the == and != operator should check for null as well like so

public static bool operator ==(Matrix obj1, Matrix obj2)
{
 if (ReferenceEquals(obj1, obj2)) { return true; }
 if (ReferenceEquals(obj1, null)) { return false; }
 if (ReferenceEquals(obj2, null)) { return false; }
 return obj1.Equals(obj2);
}
public static bool operator !=(Matrix obj1, Matrix obj2)
{
 return !(obj1 == obj2);
}

• ack: Awesome thank you @heslacher, I was just working out the need to compare to null, and your equality implementation totally makes sense! Something is funky about the static identityData solution though. See here: dotnetfiddle.net/k9aVHW I think that's because in the Matrix4 parameterless c'tor you assign data to identityData which becomes a reference. Then elsewhere, any client that mutates its copy of an identity matrix actually changes the underlying readonly(!) static data. Seems like I want to make a copy of identityData, but I'm back to needing another new for that ...