vkostyukov / la4j

Linear Algebra for Java
http://la4j.org
Apache License 2.0
373 stars 109 forks source link

Simplify the factories usage #228

Closed vkostyukov closed 9 years ago

vkostyukov commented 9 years ago

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);
SamoylovMD commented 9 years ago

Looks good, I'll get on it.

SamoylovMD commented 9 years ago

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?

vkostyukov commented 9 years ago

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.

vkostyukov commented 9 years ago

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

SamoylovMD commented 9 years ago

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?

vkostyukov commented 9 years ago

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.

SamoylovMD commented 9 years ago

Ok I'll return to generic version instead.

vkostyukov commented 9 years ago

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).
SamoylovMD commented 9 years ago

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?

vkostyukov commented 9 years ago

Looks pretty good. Please go ahed with the PR.