Introduce Optional{Filter,Map} Refaster rules
#327
Conversation
|
I'll try to circle back to this PR tomorrow to ensure that the build is running through. Local |
|
Yeah it's the |
Stephan202
force-pushed
the
werli/optional-stream-filter
branch
from
d4c6743 to
8f1b0e3
Compare
| @BeforeTemplate | ||
| Optional<? extends T> before(Optional<S> optional, Function<? super S, ? extends T> function) { | ||
| return Refaster.anyOf( | ||
| optional.stream().map(function).findAny(), optional.stream().map(function).findFirst()); |
There was a problem hiding this comment.
The second case is covered by StreamMapFirst and then OptionalIdentity, which together also yield optional.map(function).
The findAny() is not simplified because StreamMapFirst doesn't want to assume that the Stream against which it matches has a defined order. What we should perhaps do, is implement a heuristics-based Matcher for that case.
|
Thanks for the suggestions, @Stephan202. After re-thinking parts of it, I agree with your proposed changes. Let's roll with it. 👍 |
| * Avoid unnecessary {@link Optional} to {@link Stream} conversion when mapping a value of the | ||
| * former type. | ||
| */ | ||
| // XXX: If `StreamMapFirst` also simplifies `.findAny()` expressions, then this rule can be |
There was a problem hiding this comment.
IIUC in that case the self-check would identify this case right?
In that case I'd argue that it would be better to drop this XXX 😬.
@Stephan202
There was a problem hiding this comment.
I agree that we could drop it for that reason, but without it on might ask "why not add .findFirst()?" IMHO now it's clearer why this rule exists in the first place 🤷
There was a problem hiding this comment.
Okay okay. Agreed, it doesn't really fit in a @Description or something like that, so XXX is most reasonable.
rickie
force-pushed
the
werli/optional-stream-filter
branch
from
8f1b0e3 to
d88bfec
Compare
|
Rebased, will merge once 🟢. |
rickie
changed the title
OptionalStream{Filter,Map} Refaster rulesOptional{Filter,Map} Refaster rules
After I had found this construct during code review.
A mixture of
Optional#map()andOptional#filter()is however not supported with these rules. At this point it seems that Refaster won't be enough, I suspect?Suggested commit message