[
https://issues.apache.org/jira/browse/MATH-1372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15310324#comment-15310324
]
Artem Barger commented on MATH-1372:
------------------------------------
Hi,
First of all thanks for reviewing my code and writing detailed explanation.
I'm wondering what is the benefit of private static method, which basically
repeats logic of methods I've suggested and has an additional boolean parameter
which indicate whenever we would like to get min or max elements index.
While I think it terms or reducing code duplication following could have more
sense:
{code}
private static int argMinOrMax(final double[] data,
boolean isMin) {
if (data == null ||
data.length == 0) {
return -1;
}
int index = 0;
int direction = (isMin) ? 1 : -1;
double selected = direction * data[0];
for (int i = 1; i < data.length; i++) {
final double current = direction * data[i];
if (current < selected) {
index = i;
selected = current;
}
}
return index;
}
{code}
at least now it avoids code duplication at some extent.
Regarding preconditions, I've took other method of {{MathArrays}} to look what
is expected behavior for precondition testing, but didn't find something
specific hence assumed that getting an NPE is expected. Obviously I was wrong,
but that also mean methods such as: {{unique}}, {{shuffle}},
{{linearCombination}} and probably a couple of other also need to be fixed or
changed to take care of nulls and other possible incorrect inputs.
I'm sorry for the imports part, I've forgot to turn off this annoying feature
of my IDEA.
And thanks again for catching a BUG in the tests w/ Double.MIN_VALUE.
With regards to the right place, I'm not really sure, from one point of view it
make sense to keep it here, since {{MathArray}} includes a lot of utilities to
work with arrays, but I've discovered that {{StatUtils}} also includes relevant
parts as for example finding minimum or maximum value in the arrays.
> 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)
