commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (Commented) (JIRA)" <>
Subject [jira] [Commented] (MATH-677) About package "transform"
Date Wed, 30 Nov 2011 10:43:39 GMT


Gilles commented on MATH-677:

Thanks S├ębastien for having a look at this.

As I never used these classes, I couldn't come up with concrete change proposals regarding
the API.
Rather than different constructors, we could have a new class. I recall that the "solvers"
package used to contain a single "MullerSolver" class with a "solve2" method (in addition
to the "solve" defined in the interface). Now, there is a "MullerSolver2" that conforms to
the interface.

If no-one objects, you could perform that change; but I'm afraid that a thorough review of
all those classes will have to wait for 4.0. The handling of multiple dimension (through arguments
of type "Object") is particularly awkward (for a strongly typed language like Java).

In addition to the observations indicated in the issue description, I also notice a (private)
class ("MultiDimensionalComplexMatrix" in "FastFourierTransformer") that "implements" the
standard "Cloneable" interface. This is best to be avoided (cf. "Effective Java").

> About package "transform"
> -------------------------
>                 Key: MATH-677
>                 URL:
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: S├ębastien Brisard
>            Priority: Minor
>              Labels: api-change
>             Fix For: 3.0
> Classes in package "o.a.c.m.transform" might require some changes in order to conform
to goals set for the next major release.
> Some observations:
> # Exceptions
> ## Should remove use of deprecated "MathRuntimeException"
> ## Should throw more specific "Math...Exception" instances instead of standard IAE
> # Interface "RealTransformer" (and implementations) contain non-conformant method names
(e.g. "inversetransform" instead of "inverseTransform"). {color:red}Fixed in {{r1208293}}.{color}
> # "FastFourierTransformer":
> ## Methods "mdfft" and "verifyDataSet" take an argument of type "Object" (to allow an
argument with an unspecified number of dimensions)
> ## The "RootsOfUnity" helper class could be moved to the "complex" package
> ## For clarity, multidimensional transform should be moved to a class of its own (and
I also wonder whether the "MultiDimensionalComplexMatrix" name is not misleading)
> # "FastFourierTransformer", "FastSineTranformer" and "FastCosineTranformer" define public
methods "tranform2" and "inversetransform2" but they are not part of an interface
> # Code uses variables that start with an uppercase
> # "FastHadamardTransformer" contains illegible developer documentation (see Javadoc for
protected method "fht")

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:!default.jspa
For more information on JIRA, see:


View raw message