feedthemachine
feedthemachine

Reputation: 620

Return the index of the first item in generics collections

/**
 * Return the index of the first item in someCollection for which * aPredicate.test(o) is true, or -1.
 */
public static <T> int find(Collection<T> someCollection, Predicate<T> aPredicate) {
    List<T> list = new ArrayList<>();
    for (Iterator<T> iterator = someCollection.iterator(); iterator.hasNext(); ) {
        T value = iterator.next();
        if (aPredicate.test(value)) {
            list.add(value);
        }
    }
    return list[0]; // or return list.get(0)
}

With the code above, I cannot use list[0] since it needs to be replaced with list.get(0), but this method is only applicable to the collection of Integers. How can I return the index of the first element in such case?

Upvotes: 0

Views: 2096

Answers (3)

Eugene
Eugene

Reputation: 120968

Let's first make sure your method does the correct thing:

public static <T> int find(Collection<T> someCollection, Predicate<T> aPredicate) {
    int i = -1;
    for (Iterator<T> iterator = someCollection.iterator(); iterator.hasNext();) {
        ++i;
        T value = iterator.next();
        if (aPredicate.test(value)) {
            return i;
        }
    }
    return -1;
}

Let's see if it works:

public static void main(String[] args) {
    List<Integer> list = List.of(1, 2, 3);
    System.out.println(find(list, x -> x == 4)); // -1
    System.out.println(find(list, x -> x == 2)); // 1
}

It does, but is it correct?...

public static void main(String[] args) {
    Set<String> set = Set.of("abc", "ade");
    System.out.println(find(set, "abc"::equals));
}

Run this a few times (with java-9) and be surprised how the index will change. A Set does not have any order and immutable collections added in java-9 do some internal randomization when they are first created, so the results might get entirely incorrect. For a broader read, look here.

You could think that java has a common interface for when a certain Collection would be ordered or not, but it does not. So you can't really do much, other then rethink what you want to do, may be.

Upvotes: 0

Abion47
Abion47

Reputation: 24726

The purpose of your function is to return the first index of the first element in a Collection that matches the given Predicate. As such, not only should you be storing a List<int> rather than a List<T>, there's no reason to be storing a list at all if the point is to return the first thing found. As such, remove the buffer list entirely and return as soon as you find a matching element.

public static <T> int find(Collection<T> someCollection, Predicate<T> aPredicate) {
    // Collections don't necessarily natively support indices, so you must 
    // manually track the current index
    int index = 0;
    for (Iterator<T> iterator = someCollection.iterator(); iterator.hasNext(); ) {
        T value = iterator.next();
        if (aPredicate.test(value)) {
            // A matching element was found, so there's no point continuing to loop
            return index;
        }
        index++;
    }
    // No element was found, so return the conventional -1
    return -1;
}

Upvotes: 1

Louis Wasserman
Louis Wasserman

Reputation: 198211

What you want isn't to store a List<T>, but a List<Integer>, essentially, containing the index of the elements you found. You will have to track that index yourself, but tracking how many calls to iterator.next() you have made.

Upvotes: 0

Related Questions