Optimise to find objects with the highest field value in array list

Question

I made a function that returns an array list with the people having the highest number field. But I'm pretty sure that this can be optimized

public  ArrayList<Membre> findOlder(){
    ArrayList<Membre>olderPersonnes = new ArrayList<Membre>();
    int higher = 0;
    for (Membre membre : membresDeLaFamille) {
        if (membre.getNumber() > higher) {
            higher = membre.getNumber();
        }
    }
    for (Membre membre: membresDeLaFamille) {
        if (membre.getNumber() == higher) {
            olderPersonnes.add(membre);
        }
    }
    return olderPersonnes;
}

Asking for optimization for working code is a better fit for codereview.stackexchange.com. — user6073886
– user6073886, Commented Jan 3, 2022 at 16:20
@JohnnyMopp while Streams have built-in functions for "max" or "filter", they incur a performance penalty and should be avoided for simple cases where performance is an issue. — Tom Elias
– Tom Elias, Commented Jan 3, 2022 at 16:24
@TomElias This hardly depends on the use case and only the author can give us details about the size of the membresDeLaFamille collection. For better readability and maintainability i aggree with Johnny Mopp. You can exit the method immediately, if membresDeLaFamille is empty. Max might not be the correct streaming operation depending on the fact if multiple persons can have the same number assigned. — RainerZufall
– RainerZufall, Commented Jan 3, 2022 at 16:29
In terms of readability it's not great, but in terms of runtime complexity you can't have better. — plalx
– plalx, Commented Jan 3, 2022 at 16:32

Tom Elias · Accepted Answer · 2022-01-03 16:21:26Z

1

optimize? depends on what you're optimizing for.

complexity - this is an O(n) loop, it is already the least "complex" it can be. (findmax in an unsorted array of size n -> O(n))

running time - can be improved if instead of the second loop you just keep a reference to the "membre" that you find inside the first loop, and return that.

answered Jan 3, 2022 at 16:21

Tom Elias

7946 silver badges15 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

plalx Over a year ago

"instead of the second loop you just keep a reference to the "membre" that you find inside the first loop" -> What if there's more than one?

Tom Elias Over a year ago

add them to a collection like he does in the second loop. use >= instead of >

plalx Over a year ago

Gonna waste more time newing up and destroying collections than looping once again IMO. Perhaps with a linked list, but...

Tom Elias Over a year ago

IIRC, for(each) loop uses Iterator behind the scenes, that's also a waste if we're nitpicking ;)

plalx Over a year ago

Just saying that I dont think you can necessarily claim that running time will be improved with the suggestion you made :P Still +1 because I think you can't reasonably optimize further.

plalx · Accepted Answer · 2022-01-03 17:04:53Z

1

You can't do better than O(1) runtime complexity (unless the list is sorted already), so this is already optimized.

In terms of readability you could do better (arguably) using declarative (and functional concepts) vs imperative programming, but it certainly wouldn't be faster.

e.g.

public static List<Member> findOlder() {
    return membresDeLaFamille.stream()
        .map(Member::getNumber)
        .max(Integer::compare)
        .map(maxAge -> membresDeLaFamille.stream().filter(m -> maxAge.equals(m.getNumber())))
        .orElseGet(Stream::empty)
        .collect(Collectors.toList());
}

answered Jan 3, 2022 at 17:04

plalx

43.8k7 gold badges77 silver badges94 bronze badges

Comments

Nowhere Man · Accepted Answer · 2022-01-04 00:23:23Z

Basically, the list of multiple family members with the maximum number may be retrieved in the one pass if the input list membresDeLaFamille is unsorted, and as soon as the new "max" is found, current list should be cleared:

public List<Membre> findOlder() {
    List<Membre> olderPersonnes = new ArrayList<>();
    int higher = 0;
    for (Membre membre : membresDeLaFamille) {
        if (membre.getNumber() >= higher) {
            if (membre.getNumber() > higher) {
                higher = membre.getNumber();
                olderPersonnes.clear();
            }
            olderPersonnes.add(membre);
        }
    }
    return olderPersonnes;
}

If the input list happens to be already sorted by the values returned by Membre::getNumber, then the max value(s) is/are surely placed at some end of the input list (depending on the ascending or descending order of sorting).

Collectives™ on Stack Overflow

Optimise to find objects with the highest field value in array list

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related