FWIW, I like the calling pattern with the static DecompositionSolver
too...
RealVector solution =
DecompositionSolver.solve(constant, new QRDecompositionImpl(matrix));
-sujit
On Fri, 2008-12-05 at 10:37 +0100, [EMAIL PROTECTED] wrote:
> ----- "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]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
