Le 2015-10-30 10:58, Gilles a écrit :
On Fri, 30 Oct 2015 09:20:00 +0000, Eric Barnhill wrote:
On 30/10/15 02:15, Gilles wrote:
There are some problems with the Javadoc (wrong "@return" comment).
Not all local variables that are constant are declared "final".
I am happy to give it all another proof read. I take it the procedure
is to fork the dedicated branch and then submit a pull request to that
branch?
I couldn't say; we don't have a procedure yet... :-}
In fact, pull requests are really github-specific, and we don't have
a complete integration between our Apache infrastructure and github
for Commons Math. As an example, the requests are not directly
forwarded to our mailing lists so we may miss them (and we already
missed several ones ...).
One Apache motto is: "if it didn't happen on the mailing lists,
it didn't happen". So the best medium for discussion is this mailing
list. The best medium for registering issues, evolution proposals
and patches is our JIRA instance. Typically, issues start at JIRA,
are discussed here if the discussion is lengthy or need to get
public oversight, and the patches coming for non-committers are attached
to the JIRA issue (our mailing list strips attachments).
best regards,
Luc
Shouldn't independent changes be performed in separate commits?
[Referring to "IntegerSequence" and "LaguerreSolver".]
I made edits to the Solver in particular because my commit would
otherwise have left it broken.
Since you added new methods, that should not be the case.
Whenever possible, it's clearer to modify calls in other parts of the
library in a separate commit, and then remove obsolete methods in a
third step.
If that is not best practice I am happy
to revert and submit independently.
Actually, I don't like the new "size" method in "IntegerSequence".
Although the number of elements is needed in the new methods in
ComplexUtils, I don't think that we should advertise the
functionality
in its current (necessarily) poor implementation.
A better alternative is to have an instance that will hold the
number of elements it contains:
https://issues.apache.org/jira/browse/MATH-1286
My first impulse was to see if I could add a field to the range
object that contained its size. But as the method returns an Iterator
I didn't see a way to do that without returning a subclass that
extended Iterator in some way instead. I figured that was not a
desired outcome. So instead I incorporated what as far as I know is
best practice:
http://stackoverflow.com/questions/9720195/what-is-the-best-way-to-get-the-count-length-size-of-an-iterator
They don't really say it's best practice, rather, it's all you can
do with an Iterator. ;-)
Alternatively I could just create a local private method in
ComplexUtils that determined the size by iterating through the range
and call that, and leave IntegerSequence alone.
When MATH-1286 is resolved, that won't be necessary.
Best regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
