4
\$\begingroup\$

I have an array and need to identify minimum value in the array - This is easy. However, this array can have duplicates and there exist multiple minimum values and we need to identify them.

The following code works and is done through 2 loops. First loop identifies the minimum value and the second loop all values which equate to the minimum value.

Please let me know your thoughts and if any other efficient way is possible.

public static List<Integer> getMinIndex(double... arrayList){
    int minIndex = 0;
    int i = 0; 
    int len = arrayList.length;
    while(i < len){
        if(arrayList[i] < arrayList[minIndex]  )
            minIndex = i;
        i++;
    }

    //Below is specifically for case where are multiple minimums and we want all of them
    i=0;
    List<Integer> minList = new ArrayList<Integer>();
    while(i < len){
        if(arrayList[i] == arrayList[minIndex]  )
            minList.add(i);
        i++;
    }
    logger.debug("Postition of min-item in arraylist {}",Arrays.toString(minList.toArray()));
    return minList;
}
\$\endgroup\$
4
\$\begingroup\$

You can achieve what you want, just with one for loop:

int minIndex = 0;
List<Integer> list = new ArrayList<>();

for (int i = 0; i < array.length ; i++) {
    if (array[i] == array[minIndex]) {
        list.add(i);
    } else if (array[minIndex] > array[i]) {
        list.clear();
        list.add(i);

        minIndex = i;
    }
}

return list;

Changes and recommendations:

  1. I think that you must change the name of the method variable to 'array', since there is a Java collection class named ArrayList and it could confuse some readers of your code
  2. You should also change the method name, since from its name it is not clear that it returns a list of min indexes. Maybe getMinIndexes could be used
  3. Regarding the old code, I think that you should use less temporal variables, in your case len variable, doesn't give any more semantic to the code, so it could be removed. When you works with arrays consider the usage of for loops.

Update

As additional recommendations I would add:

  1. I think that 'find' could be used as prefix instead of 'get', since you are actually searching for those indexes, you don't getting them from some POJO
  2. I also think that it is better if you are using curly brackets even in a single line block statements
  3. Also there is no need of transforming the list into array and then into string in the debug log. Lists has nice implementation of toString function that would print the elements the same way as Arrays.toString
\$\endgroup\$
3
  • \$\begingroup\$ Good answer. I'd just get rid of minIndex/ array[minIndex] and track the minimal value directly instead starting with double minValue = Double.MAX_VALUE;. \$\endgroup\$ – RoToRa Aug 28 '17 at 12:17
  • \$\begingroup\$ Elegant. While this is great, I sometimes do get a feeling that my solution while resource intensive is quite easy for new viewers to understand and come upto speed. Am i wrong, would love your thoughts. \$\endgroup\$ – HopeKing Aug 28 '17 at 13:44
  • \$\begingroup\$ Maybe your solution is really easier for the new readers to get used to, since I had merged two operations into a single loop. If efficiency is not important, then you could stay with your solution and just extract the first operation in separate function, but instead of finding the minimum index, find the minimum value, this will save you one additional access to the array by index. I also will update the comment with some additional recommendations \$\endgroup\$ – RaISug Aug 28 '17 at 16:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.