vkostyukov/la4j

Simplify the factories usage

vkostyukov opened this issue · 10 comments

We can get rid of all these annoying methods like: Matrix foo(Something something, Factory factory), just always using the current factory associated with this object. In order to convert the things we can add the following methods:

interface Matrix {
  DenseMatrix toDenseMatrix(); // use Basic2D
  SparseMatrix toSparseMatrix(); // use CRS
  Matrix to(Factory factory);
}

Also we can use different names for factories in LinearAlgebra object to make the code more redable. Like this:

Matrix a = ???
DenseMatrix b = a.toDenseMatrix();
Matrix c = a.to(LinearAlgebra.CCS);

Looks good, I'll get on it.

My first thought is to make something like this:

//in AbstractMatrix
public Matrix to(Factory factory) {
    return factory.createMatrix(this);
}
public DenseMatrix toDenseMatrix() {
    return to(LinearAlgebra.DENSE_FACTORY);
}
public SparseMatrix toSparseMatrix() {
    return to(LinearAlgebra.SPARSE_FACTORY);
}

So, is that all?

Yeah. Right. But I would also like to see that to method returns type-safe matrix (DenseMatrix or SparseMatrix). You can achieve it having a generic method to. Like this:

interface Matrix {
<T> T to(MatrixConverter<T> );
}

Where MatrixConverter is:

interface MatrixConverter<T extends Factory> {
  T convert(Matrix matrix);
}

// in LinearAlgebra
MatrixConverter<DenseMatrix> BASIC_2D = new MatrixConverter<DenseMatrix> {
  DenseMatrix convert(Matrix matrix) {
    return BASIC2D_FACTORY.create(matrix)l
  }
}

// in AbstractMatrix
public <T> T to(MatrixConverter<T> converter) {
  return converter.convert(this);
}

Also feel free to use other name instead of MatrixConverter. Not sure if it fit well.

And we also probably want to mark all the methods that take factory explicitly @deprecated.

So, after some thinking I suggest the following syntax:

interface Matrix {
    public Matrix to(Class clazz);
}

...

Matrix b = a.to(Basic1DMatrix.class);

This looks like enterprise feature! However, it uses reflection and can slow down performance, but looks pretty. What do you think?

That doesn't look safe. You can pass anything into to method. And also reflection is the last thing I wanted to use in math package :) Let's go with interfaces and classes.

Ok I'll return to generic version instead.

We can also consider naming the convert method as instead of to. My thoughts is to converts one object to other, a different object (changing it's type, shape, behavior). We do have. toRowVector(), etc. And it looks pretty good. In case of converting matrices to different type, we doesn't change it's shape: it's still a matrix with the same behavior. We can name that method as to indicate that we doesn't change the object type.

Matrix x = ???
Matrix y = x.as(LinearAlgebra.CRS).

So, I started to work on this.

Some code examples:

//Matrices.java
    public static final Factory BASIC2D_FACTORY =
            LinearAlgebra.BASIC2D_FACTORY;

    public static final MatrixConverter<Basic2DMatrix> BASIC_2D =
            new MatrixConverter<Basic2DMatrix>() {
                @Override
                public Basic2DMatrix convert(Matrix matrix) {
                    return (Basic2DMatrix) BASIC2D_FACTORY.createMatrix(matrix);
                }
            };

//MatrixConverter.java
public interface MatrixConverter<T extends Matrix> {
    T convert(Matrix matrix);
}

What do you think about it?

Looks pretty good. Please go ahed with the PR.