Introduce Flux and Stream Refaster rules to suggest filtering before sorting
#393
Conversation
|
Looks good. No mutations were possible for these changes. |
4 similar comments
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
|
Looks good. No mutations were possible for these changes. |
|
Rebased to test whether I can push to the fork, which worked ✔️ Adding the PR to my review list :) |
|
Looks good. No mutations were possible for these changes. |
| @@ -167,6 +167,34 @@ Stream<R> after(Stream<T> stream, Function<? super S, ? extends Stream<? extends | |||
| } | |||
| } | |||
|
|
|||
| /** Apply filtering before sorting to reduce the number of elements to sort. */ | |||
There was a problem hiding this comment.
Perhaps slightly more idiomatic:
| /** Apply filtering before sorting to reduce the number of elements to sort. */ | |
| /** Filter before sorting to reduce the number of elements to sort. */ |
There was a problem hiding this comment.
Alternatively:
| /** Apply filtering before sorting to reduce the number of elements to sort. */ | |
| /** Apply {@link Stream#filter(Predicate)} before {@link Stream#sorted()} to reduce the number of elements to sort. */ |
In most cases we do reference the relevant methods, so let's go with this one.
| @@ -167,6 +167,34 @@ Stream<R> after(Stream<T> stream, Function<? super S, ? extends Stream<? extends | |||
| } | |||
| } | |||
|
|
|||
| /** Apply filtering before sorting to reduce the number of elements to sort. */ | |||
| static final class SortAfterFilter<T> { | |||
There was a problem hiding this comment.
(We still have to automate this, but) our current naming scheme would dictate that this class is named StreamFilterSorted.
|
|
||
| Stream<Integer> testSortWithComparatorAfterFilter() { | ||
| return Stream.of(1, 4, 3, 2) | ||
| .sorted(Comparator.comparingInt(Integer::intValue).reversed()) |
There was a problem hiding this comment.
If we use reverseOrder() we can simplify elidedTypesAndStaticImports above.
|
Looks good. No mutations were possible for these changes. |
1 similar comment
|
Looks good. No mutations were possible for these changes. |
There was a problem hiding this comment.
Added one more commit. Very nice contribution @CoolTomatos!
Suggested commit message:
Introduce `Flux` and `Stream` Refaster rules to suggest filtering before sorting (#393)
Fixes #386.
| @@ -1246,4 +1247,36 @@ Duration after(StepVerifier.LastStep step, Duration duration) { | |||
| return step.verifyTimeout(duration); | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Right now we have the StepVerifier-related rules at the bottom; let's keep that.
|
Looks good. No mutations were possible for these changes. |
There was a problem hiding this comment.
Rebased and added a commit.
Changes LGTM!
Made one tweak to the suggested commit message to be more inline with the previous commit messages (IMO).
Thanks for opening a PR Sir @CoolTomatos 😉!
| * Apply {@link Flux#filter(Predicate)} before {@link Flux#sort()} to reduce the number of | ||
| * elements to sort. | ||
| */ | ||
| static final class StreamFilterSort<T> { |
There was a problem hiding this comment.
This looks like a small copy / paste error from the actual StreamRules. This should be FluxFilterSort, likewise for the WithComparator one :).
|
Looks good. No mutations were possible for these changes. |
|
Rebased, will merge once 💚. |
|
Looks good. No mutations were possible for these changes. |
rickie
changed the title
Flux and Stream Refaster rules to suggest filtering before sorting
Closes #386.