my 'smallest method gives different results if order of when the words/numbers are added is changed

Question

So basically my code is doing what the question says. In the way the code is laid out now it gives the correct results, but when I change the order of the .add pieces of code it gives different results each time. I feel the compareTo method is fine, but am i missing something? I'm trying to get the smallest result.

Thanks in advance.

package lists;

import java.util.*;

public class Lab4 {
    public static <T extends Comparable> int smallest(List<T> l) {
        if (l.size() == 0) 
            return -1;
        else {
            Iterator<T> it = l.iterator();
            T smallestSoFar = it.next();
            T temp;

            int smallestPos = 0;
            int i = 0; //used to indicate position in list of next item
            while (it.hasNext()) {
                temp = it.next();

                if (temp.compareTo(smallestSoFar) > 0) {
                    smallestSoFar = temp;
                    smallestPos++;
                }
                i++;
            }

            return smallestPos;
        }
    }

    public static <T extends Comparable> void deleteSmallest(List<T> l) { // for exercise 3
    }

    public static void main(String[] args) {
        Vector<String> vec1 = new Vector<String>();
        vec1.add("Hello");
        vec1.add("xxxx");
        vec1.add("world");
        vec1.add("aardvark");

        int smallPos = smallest(vec1);
        if (smallPos != -1)
            System.out.println("smallest entry is " + vec1.elementAt(smallPos) + " at position " + smallPos);
        Vector<Integer> vec2 = new Vector<Integer>();

        vec2.add(new Integer(47));
        vec2.add(new Integer(247));
        vec2.add(new Integer(17));
        vec2.add(new Integer(399));

        smallPos = smallest(vec2);
        if (smallPos != -1)
            System.out.println("smallest entry is " + vec2.elementAt(smallPos) + " at position " + smallPos);
    }
}

Answer 1

Your comparison test is the wrong way around. Currently you're picking the largest value.

if (temp.compareTo(smallestSoFar) > 0) {

Should be

if (temp.compareTo(smallestSoFar) < 0) {

Also, smallestPos++;should be smallestPos=i;

Currently you're returning a count of the number of times the "smallest" value changed.