(I'm writing about matrices, but don't worry, this is not a maths question and presumes no maths knowledge.)

I have a Matrix class that has three fields

double[] Data;
int Rows;
int Columns;

and defines hundreds of mathematical operations.

Additionally, I have a SymmetricMatrix : Matrix subclass that has no instance fields of its own, that offers all of the operations above via inheritance, offers some additional operations (like EigenvalueDecomposition), and finally repeats some definitions with a new keyword to change the return type to SymmetricMatrix.

For example, if Matrix has

public Matrix MatrixMultiplication(Matrix otherMatrix)
public Matrix ScaledBy(double scalar)

then SymmetricMatrix would simply inherit the MatrixMultiplication but it would change the ScaledBy to return a SymmetricMatrix.

Rather than reimplementing ScaledBy , I define it via

// SymmetricMatrix.cs
public new SymmetricMatrix ScaledBy(double scalar) => base.ScaledBy(scalar).AsSymmetricMatrix()
// Matrix.cs
public SymmetricMatrix AsSymmetricMatrix() => Unsafe.As<SymmetricMatrix>()

(I'm using new instead of virtual override for reasons that don't matter for the purposes of this question).

I found this approach to work surprisingly well, and it allows me to be super succinct in defining SymmetricMatrix.cs. The obvious downside is that it may exploit unsupported behavior (?) and that it confuses the debugger I'm using a lot (the runtime type of the result of the cast is Matrix which is not a subclass of its compile time type SymmetricMatrix, and yet, all the operations defined on SymmetricMatrix succeed because the data held by both classes is the same)

Questions

1. Do you foresee any problems with my approach that I haven't though of?

2. Might my approach break with the next dotnet version?

3. Do you think there are cleaner ways of achieving what I want? I had a few ideas but none seem to work out cleanly. For example, I cannot encode which operations preserve Child classes in the parent class via

class Matrix<T> where T : Matrix
...
T ScaledBy(double other)

because whether or not an operation preserves a Child class is knowledge only the Child can have. For example, SymmetricPositiveDefiniteMatrix would NOT be preserved by ScaledBy.

Alternatively, I could use encapsulation over inheritance and define

class BaseMatrix
{
    ...lots of operations
}
...
class Matrix : BaseMatrix
{
    private BaseMatrix _baseMatrix;
    ...lots of "OperationX => new Matrix(_baseMatrix.OperationX)"
}
class SymmetricMatrix : BaseMatrix
{
    private BaseMatrix _baseMatrix;
    ...lots of "OperationX => preserving ? new SymmetricMatrix(_baseMatrix.OperationX) : new Matrix(_baseMatrix.OperationX);"
}

That's very sad to code up though, because I'd have to manually propagate every change I make to BaseMatrix to, at the moment, four extra classes, and users would have to manually cast SymmetricMatrixs to Matrix in lots of scenarios.

Finally, I could simply not offer subclasses and rather have flags like bool _isSymmetric and encode the operations that preserve symmetry/positivity/... by changing/setting the flags in all my methods. This is sad too, because :

1. Users would then be able to write Matrix.EigenvalueDecomposition() just to have this break at runtime with a You have to promise me that your matrix is symmetric because I only implemented EigenvalueDecomposition for that case error

2. It would be too easy to forget resetting a flag when it's not preserved and then accidentally running an operation that assumes symmetry (which BLAS does by simply ignoring half of the matrix)

3. I like being able to specify in the type system rather than the comments that, e.g., the matrix I return from CholeskyFactor is lower triangular (conventions differ and some may expect an upper triangular matrix)

4. Users wouldn't see which flags are currently set so they wouldn't know whether I use the "specialized" version of a given algorithm and likely end up using .SetSymmetric() unnecessarily all over the place just to be safe.

CodePudding user response：

Unless I missed a requirement, you could implement what you want using generics

A (not so brief) example :

public abstract class BaseMatrix<TMatrix> where TMatrix : BaseMatrix<TMatrix>
{
    // Replace with how you actuallt are storing you matrix values
    protected object _valueStore;

    // common initialization (if required), if not keep empty
    protected BaseMatrix()
    {

    }

    // necessary to copy the value store.
    protected BaseMatrix(object valueStore)
    {
        this._valueStore = valueStore;
    }

    public virtual TMatrix ScaledBy(double scalar) 
    {
        // implementation
        return default;
    }
}

public class Matrix : BaseMatrix<Matrix>
{

}

public class SymmetricMatrix : BaseMatrix<SymmetricMatrix>
{
    public SymmetricMatrix() : base(){}

    // allows to build from another matrix, could be internal
    public SymmetricMatrix(object valueStore) : base(valueStore){}

    public override SymmetricMatrix ScaledBy(double scalar)
    {
        return base.ScaledBy(scalar);
    }
}

public class SymmetricPositiveDefiniteMatrix : BaseMatrix<SymmetricPositiveDefiniteMatrix> 
{
    // If we *know* the type is not the same after this method call, we mask with a new one retirning the target type
    // A custom cast operator will handle the conversion
    public new SymmetricMatrix ScaledBy(double scalar)
    {
        return base.ScaledBy(scalar);
    }

    // Added to allow a cast between SymmetricPositiveDefiniteMatrix  and SymmetricMatrix
    // in this example, we keep the value store to not have to copy all the values
    // Depending on project rules could be explicit instead
    public static implicit operator SymmetricMatrix(SymmetricPositiveDefiniteMatrix positiveSymmetricMatrix)
    {
        return new SymmetricMatrix(positiveSymmetricMatrix._valueStore);
    }
}

Which would make the ScaledBy(double) method available in each type of matrix, and returning the type of the matrix it was called on.

And it would comply with your requirement not to have to redefine most methods / properties in each class.

Obviously, you can override methods in childrens to either do more before or after the base() call or even define an entirely different implementation.

One shortcoming would be if you needed one type to inherit from a class inheriting from BaseMatrix<TMatrix>. You'd have no way of changing the generic type parameter.

Alternatively if this really doesn't match your needs and prefer to go the encapsulation route, you could have a look at source generators since the code seems to be quite repetitive.

CodePudding user response：

I think that the use of Unsafe.As() is indeed unsafe.

A major problem is this:

Matrix mat = new Matrix(/* whatever */);
SymmetricMatrix smat = mat.AsSymmetricMatrix();
Console.WriteLine(smat.GetType().FullName); // "Matrix"

So you have something declared as SymmetrixMatrix but its underlying type is in fact Matrix. I'm sure you can imagine the sort of horrible problems that could cause...

This approach also requires that the base class knows about its derived classes - a big no-no.

One way to solve issues like this it to make sure you can clone your objects in a way that works with inheritance. Then you can clone the original objects and return them, with a cast if necessary.

For example:

public class Matrix
{
    public Matrix()
    {
        // Whatever
    }

    // Copy constructor; used for cloning.
    public Matrix(Matrix other)
    {
        _data = other._data.ToArray();
    }

    public virtual Matrix ScaledBy(double scalar)
    {
        var result = Clone();
        result.ScaleBy(scalar);
        return result;
    }

    protected void ScaleBy(double scalar)
    {
        for (int i = 0; i < _data.Length;   i)
        {
            _data[i] *= scalar;
        }
    }

    protected virtual Matrix Clone()
    {
        return new Matrix(this);
    }

    readonly double[] _data = new double[16]; // For illustration.
}

public class SymmetricMatrix: Matrix
{
    public SymmetricMatrix()
    {
        // Whatever
    }

    // Copy constructor; used for cloning.
    public SymmetricMatrix(SymmetricMatrix other): base(other)
    {
        // No new fields to copy.
        // Any newly added fields should be copied here.
    }

    public override SymmetricMatrix ScaledBy(double scalar)
    {
        // This cast will work because the underlying type returned from 
        // base.ScaledBy() really is SymmetricMatrix
        return (SymmetricMatrix)base.ScaledBy(scalar);
    }

    protected override SymmetricMatrix Clone()
    {
        return new SymmetricMatrix(this);
    }
}

public class SymmetricPositiveDefiniteMatrix: Matrix
{
    // Doesn't override ScaledBy() so SymmetricPositiveDefiniteMatrix.ScaledBy() will return a Matrix.

    protected override SymmetricPositiveDefiniteMatrix Clone()
    {
        // If SymmetricMatrix ever adds new fields to be cloned, they should be cloned here.
        return Unsafe.As<SymmetricPositiveDefiniteMatrix>(base.Clone());
    }
}

Now code like this returns the expected types:

var smatrix = new SymmetricMatrix();
var sresult = smatrix.ScaledBy(1.0);

Console.WriteLine(sresult.GetType().FullName); // "SymmetricMatrix"

var spmatrix = new SymmetricPositiveDefiniteMatrix();
var spresult = spmatrix.ScaledBy(1.0);

Console.WriteLine(spresult.GetType().FullName); // "Matrix"

As an example of code where Unsafe.As() can cause strange results, consider this code:

using System;
using System.Runtime.CompilerServices;

namespace ConsoleApp1;

static class Program
{
    public static void Main()
    {
        var myBase = new MyBaseClass();
        var pretendDerived = myBase.AsMyDerivedClass();

        // Given the definition of MyDerivedClass, what do you think this will print?
        Console.WriteLine(pretendDerived.Name());
    }
}

public class MyBaseClass
{
    public MyDerivedClass AsMyDerivedClass() => Unsafe.As<MyDerivedClass>(this);
}

public class MyDerivedClass
{
    readonly string _name = "TEST";
    public string Name() => _name;
}

As the documentation for Unsafe.As() states:

The behavior of Unsafe.As(o) is only well-defined if the typical "safe" casting operation (T)o would have succeeded. Use of this API to circumvent casts that would otherwise have failed is unsupported and could result in runtime instability.