Convert stateful predicate distinctByKey into a list method
#5462
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #5462 +/- ##
=============================================
- Coverage 66.94% 66.80% -0.14%
- Complexity 15546 15560 +14
=============================================
Files 1802 1805 +3
Lines 69917 69839 -78
Branches 7360 7358 -2
=============================================
- Hits 46805 46659 -146
- Misses 20650 20723 +73
+ Partials 2462 2457 -5
☔ View full report in Codecov by Sentry. |
|
Question: There is a few things in here witch might be slower that the original code - making lists and throwing them away - but I am not sure if it matters. Is the performance important enough, and does this code run once or very often(more GC)? |
|
I can reduce the number of collections created but it won't be quite as efficient as it was previously:
I don't think it matters a lot either way with the current uses but the library code should probably be as efficient as possible. |
|
There is now a JEP which adds "gatherers" to the Stream API allowing you to implement a It's even the example used in the document. |
| void distinctByKey() { | ||
| var first = new Wrapper(10, makeHello()); | ||
| var last = new Wrapper(20, "HI"); | ||
| var duplicates = List.of(first, new Wrapper(20, makeHello()), last); | ||
|
|
||
| var deduplicated = ListUtils.distinctByKey(duplicates, w -> w.string); | ||
|
|
||
| assertEquals(List.of(first, last), deduplicated); | ||
| } |
There was a problem hiding this comment.
I find it slightly difficult to understand what this test does.
There was a problem hiding this comment.
I've tried to improve the readability of the test and added comments explaining what is being tested.
Summary
In a previous PR I used a stateful predicate to de-duplicate a stream by a key of the list elements. @vpaturet has correctly flagged this as problematic.
This PR converts so that it operates on
Listrather thanStreambecause an intermediate collection was required anyway.Unit tests
Moved.