ArrayList to Stream in Java by grouping

Question

I would like to get the highest score group by Id .If two highest score's are same then i would like get the highest score based on lowest Optional ID.I would like to get it in Java Stream.So far this code works.Is there any efficient way to rewrite this code in java stream

Example :

records=record.Person(batchNumber);
List<Person> highestRecords = new ArrayList<>();for(
Person s:records)
  {
if(!highestRecords.isEmpty()) {
    boolean contains = false;
    for(Person ns: new ArrayList<>(highestRecords)) {
        if(s.Id().compareTo(ns.Id()) == 0) {
            contains = true;
            if(s.getScore.compareTo(ns.getScore()) > 0     
    && s.optionalId().compareTo(ns.optionalId()) < 0) {

                highestRecords.remove(ns);
                highestRecords.add(s)       
            }
        }
    }
    if(contains == false) {
        highestRecords.add(s);
    }
}else {
    highestRecords.add(s);
}
  }
}

okay we see you have code using for each loop, why don't you explain with some input and output example — Ryuzaki L
– Ryuzaki L, Commented Jan 25, 2019 at 17:10
Variable names are really confusing, why List<Person> called getNewPendingMatches? Maybe the first step should be to clean-up existing solution before thinking about conversion to FP. — J-Alex
– J-Alex, Commented Jan 25, 2019 at 17:12
Just so you know, I do see the edits you're making to your question. If you're hoping that may entice someone to come along to write this out as a stream, that's...unlikely. — Makoto
– Makoto, Commented Jan 25, 2019 at 17:38
Thanks i thought my question was not clear thats why i made some changes — user2410266
– user2410266, Commented Jan 25, 2019 at 17:41
Off topic but I would strongly suggest you look over your naming practices of variable because this code is very hard to read. Don't name a variable getX because this name suggests a method that will return a class member so it gets confusing reading it. Furthermore separate the names better, having variables with long almost identical names also makes it harder to understand the code. — Joakim Danielson
– Joakim Danielson, Commented Jan 25, 2019 at 17:43

Makoto · Accepted Answer · 2019-01-25 17:19:38Z

3

Don't convert this to a stream.

There is no one pure operation happening here. There are several.

Of note is the initial operation:

if(getNewPendingMatches.size() > 0)

That's always going to be false on the first iteration and you're always going to add one element in.

On subsequent iterations, life gets weird because now you're trying to remove elements while iterating over them. A stream cannot delete values from itself while iterating over itself; it only ever processes in one direction.

As written this code should not be converted to a stream. You won't gain any benefits in doing so, and you're going to actively harm readability if you do.

answered Jan 25, 2019 at 17:19

Makoto

107k29 gold badges199 silver badges236 bronze badges

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

6 Comments

Mark Over a year ago

.... "On subsequent iterations, life gets weird" - very much agreed. Not sure the code could get less readable than it is already!

Makoto Over a year ago

@MarkKeen: You know what would make it more unreadable? Converting it into a stream. ;)

Mark Over a year ago

A stream would be very readable .... based on the example requirement, not supplied code....

Makoto Over a year ago

Meh - I can't see much way around the forEaches required to glue it all together, honestly...

Mark Over a year ago

ha .. as I say based on the example output, not supplied code .. as per my comment on OP's question ..

|

Collectives™ on Stack Overflow

ArrayList to Stream in Java by grouping

1 Answer 1

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related