----- "Phil Steitz" <[EMAIL PROTECTED]> a écrit :
> Looks good. Just one last question / suggestion. In the
> DecompositionSolver, why to we keep the argumentless decompose()
> methods
> and the matrix as an instance variable? The argumentless methods just
>
> delegate to the ones that take a matrix as an argument and the
> instance
> variable is otherwise unused.
>
> Phil
This also bothers me. I tried different things and ended up with that but
didn't feel comfortable with it. Since it was already very late, I commited it
for review.
In addition to the fact the instance field is almost not used, the calling code
is awkward:
DecompositionSolver solver = new DecompositionSolver(matrix);
QRDecomposition decomposition = solver.qrDecompose();
RealVector solution = solver.solve(constant, decomposition);
I would prefer a one-liner. After one night sleeping, I see two better
approaches. The first one would be to remove completely the field instance, the
xyzDecompose methods and change all other methods (solve, isNonSingular ...) to
static, thus making DecompositionSolver a utility class never instantiated. The
second one would be to have a DecompositionSolver that can be instantiated but
instead of storing the matrix, it would store a configuration flag specifying
which decomposition algorithm to use, in this case, the undecomposed matrix
would be passed in the solve method and decomposed on the fly.
The first approach leads to:
RealVector solution =
DecompositionSolver.solve(constant, new QRDecompositionImpl(matrix));
The second approach leads to:
RealVector solution =
new DecompositionSolver(DecompositionSolver.USE_QR).solve(constant, matrix);
I prefer the first one. The decomposition algorithm is more explicit to users
and they can reuse it. The second approach only advantage is if someone wants
to configure a solver with some algorithm type once and for all and to reuse it
several time in his code. However I think this use case is not a common one and
it could be done by a suitable wrapper.
There is also one other thing I don't really like in my current implementation:
the overloaded methods with the four types of decomposition
(EigenDecomposition, SingularValuesDecomposition, QRDecomposition,
LUDecomposition). However, simplifying this would require reintroducing some
common base interface and wrappers like this:
public class Solver {
public RealVector solve(RealVector constant) {
...
}
}
public class QRSolver implements Solver {
...
}
public class DecompositionSolver {
public RealVector solve(RealVector constant, Solver solver) {
return solver.solve(constant);
}
}
and we would end up with a DecompositionSolver that has no purpose at all! So I
think it is better to stick with the four signatures types.
I will therefore implement and commit today the first approach without the four
signatures types, just to see how it looks once implemented. It is
straightforward and could be changed afterwards if something better is found.
Luc
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
