[
https://issues.apache.org/jira/browse/MATH-1372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15310244#comment-15310244
]
Gilles commented on MATH-1372:
------------------------------
Hi Artem.
I've just created a branch "feature-MATH-1372".
You'll notice that the committed code is not exactly what your patch contains.
It's a way to illustrate the level of coding practice which we try to maintain
(please review my changes and ask questions if something is unclear or looks
wrong)
In particular:
* Everything must be fully documented
* Preconditions must be handled (and tested)
Often a failed precondition would raise an exception (in this case, it
doesn't in my version, but it's TBD)
* Changes that are unrelated to the issue must be avoided (like reordering of
"import" statements)
Side note: in the patch, in unit test method "testArgMaxNegativeOnlyValues",
the tested array contained a positive value
([Double.MIN_VALUE|https://docs.oracle.com/javase/7/docs/api/java/lang/Double.html#MIN_VALUE]).
It is nit-picking, yes, but
* in a collaborative environment, it helps reducing the amount of "noise" (to
make it easier for a reviewer to understand what is being done).
* it makes the code more robust and understandable in the long term
Concerning the feature itself, we must consider that a class such as
{{MathArrays}} is mostly intended to contain helpers that are used within the
library.
Is this the case here?
> Add argmin/argmax static functions to MathArrays utility class
> --------------------------------------------------------------
>
> Key: MATH-1372
> URL: https://issues.apache.org/jira/browse/MATH-1372
> Project: Commons Math
> Issue Type: Wish
> Reporter: Artem Barger
> Assignee: Artem Barger
> Priority: Trivial
> Attachments: MATH-1372.patch
>
>
> Following conversation in the ML thread.
> Working lately w/ CM, I've found myself using a lot functions which helps to
> identify the index of the min/max elements in given array of doubles.
> In ML it was pointed out that similar functionality already exists in
> RealVector, however due to MATH-765 it's planned to be removed (deprecated),
> moreover to use it one has to create an instance of RealVector, where for
> simple cases it might be redundant.
> Hence I'd like to propose to add these static methods to the MathArrays
> utility class.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
