Phil Steitz wrote:
Phil Steitz wrote:
Thanks, Bernhard for the contribution in MATH-236. I would like to
suggest a couple of improvements.
First, I think the return type should be List, not Collection, as
there is an order to the elements in the returned collection (as
stated in the API doc, the order is by increasing argument).
Second, since we are also making the method that returns the
argument public, I would prefer to name that "getArgument"
(preferred) or "getArg" instead of "getPhi".
Finally, in the loop that generates the roots, it would be better to
compute the pie slice once and then add it each time instead of
computing k* 2 * Math.PI/ n for each k > 1.
If there are no objections, I will make these changes.
Thanks again for the contribution.
Phil
I made the changes above, but as I was updating the tests, I noticed
another thing. Behavior for complex numbers with NaN and infinite
parts is, well, "interesting." It is consistent with what we do
elsewhere in this class to just apply computational formulas and
document behavior for infinite and NaN; but the current implementation
is hard to document correctly and the results are a little strange for
infinite values.
I see three reasonable choices here, and am interested in others'
opinions. Please select from the following or suggest something else.
1) Leave as is and just point out that the computational formula will
behave strangely for infinite values.
2) return {Complex.NaN} or {Complex.INF} respectively when the number
has a NaN or infinite part.
3) return {Complex.NaN} when either part is NaN; but for infinite
values, compute the argument using getArgument (atan2), generate the
arguments for the roots from this and select the real/im parts of the
roots from {0, inf, -inf} to match the argument (this will always be
possible because atan2 always returns a multiple of pi/4 for infinite
arguments). For example, the 4th roots of real positive infinity
would be {inf + 0i, 0 + infi, -inf + 0i, 0 + -infi}
2) is probably the most defensible mathematically; but 3) is closer to
the spirit of C99x. Unfortunately, since our implementation of
multiply is 2)-like, 3) does not guarantee that nth roots actually
solve the equation r^n = z.
Phil
Sorry, just realized that 3) will not work in general, so choice is 1),
2) or a better idea.
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
