finding the minimum of an array

Question

I'm trying to find the minimum of an array with ten inputted points, but I somehow have managed to create something that will only find the maximum. Help?

import java.util.Scanner;

public class Ex7_9Smallestt {

    public static void main(String[] args) {

        Scanner input = new Scanner(System.in);
        int count = 0;

        System.out.print("Welcome to Elisabeth's smallest number finder!\n");
        //print welcome message

        double myList [] = new double[10]; //initialize array

        while (count < 10) { //initialize for loop
            //print enter a number and make that number an element in the array
            int i = 0;
            System.out.print("\nPlease enter a number:");
            myList[i] = input.nextDouble();
            count ++;
        }

        System.out.printf("The minimum is " + min(myList)); //print minimum
    }

    public static double min(double[] array) {
        //create method to find lowest number

        double minimum = array[0];
        int i = 0;

        //initialize for loop
        for (double e : array) {
            if (array[i] < minimum) {
                minimum = array[i];
                i++;
            }
        }
        return minimum;
    }
}

Why are you mixing indices and the collections for loop in the min method? Why not just iterate through the array and forget the indices? — ncmathsadist, Dec 01 '15 at 02:24

score 2 · Answer 1 · answered Dec 01 '15 at 02:06

for(double e: array){
    if(array[i] < minimum){
        minimum = array[i];
    } 
    i++;
}

Above would have worked while yours below doesn't.

for(double e: array){
    if(array[i] < minimum){
        minimum = array[i];
        i++;
    } 
}

The reason is simple. You don't increment i in all cases and so it gets stuck and compares the same element over and over again. It's not returning the maximum, it's most of the time the first element.

You don't need i though since you use an enhanced for loop and that means the right approach would be

for(double e: array) {
    if(e < minimum) {
        minimum = e;
    } 
}

Or if you like the i

for (int i = 0; i < array.length; i++) {
    if (array[i] < minimum) {
        minimum = array[i];
    }
}

Combining both loop types works but it feels more like that's by accident.

score 1 · Answer 2 · answered Dec 01 '15 at 02:01

If your using a for loop like that then

for ( double candidate : array ) {
  if ( candidate < minimum ) {
    minimum = candidate;
  }
}

is better than mixing the access by index.

@Test public void min() {
    double[] array = new double[4];

    array[0] = 5;
    array[1] = 2;
    array[2] = 3;
    array[3] = 7;

    double min = array[0];

    for ( double candidate : array ) {
        if ( candidate < min ) {
            min = candidate;
        }
    }

    assertEquals(2, min, 0.05);
}

your example is failing though because your only incrementing i if the first element is actually less that itself (ie never) ... check the first iteration through the loop by working it out on paper.

with your code this would work

for(double e: array){
    if(array[i] < minimum){
        minimum = array[i];

    }
    i++;
}

but you can see that not mixing the access by index is much more readable, and well less prone to errors.

score 0 · Answer 3 · answered Dec 01 '15 at 01:51

Firstly, you fill your array incorrectly. Currently you have

int i = 0;
System.out.print("\nPlease enter a number:");
myList[i] = input.nextDouble();
count ++;

occurring on each iteration. So, you're setting i to zero each time and storing myList[i] every time, so you're just filling myList[0] over and over again. Instead, loop until count < myList.length and use myList[count] to assign a value at each index.

Second of all, you use a for-each loop for no reason in your minimum finder method. I would change that to just a normal for loop since you use i to index into your array anyway.

Mat Jones · Answer 4 · 2015-12-01T02:16:42.363

First off, it's easier and looks much nicer to simply use the println() method rather than print with a \n character.

Also, why not just prompt the user Enter 10 values: and use a for loop for int i<10?

In min() you are essentially creating an ad-hoc for loop within an enhanced for loop. This is unnecessary. Get rid of the int i completely. Also, you are only sorting relative to the next element. You need to check that it is the absolute minimum.

I would switch from using an array to an ArrayList<int>. Note: everything in the <> is a type parameterizer; typically, you would use E as a placeholder, making it a generic algorithm (the method can work with any data type that extends the Comparable interface). That way, you can use an iterator, and use the more efficient algorithm:

import java.util.Collection;
import java.util.Iterator;
import java.util.List;

public static<Integer extends Comparable<Integer>> Integer findMin(Collection<Integer> c) {
    Iterator<Integer> i=c.iterator();
    Integer current=i.next();
    Integer min=current;
    Integer next;

    while(i.hasNext()){
        next=i.next();
        if(min.compareTo(next)>0){
            min=next;
        }
    }
    return min;
}

Edit: Sorry, I misread your code; just replace every instance of Integer in my code with double in yours.

finding the minimum of an array

4 Answers4