Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

StreamMatchers.empty() and StreamMatchers.equalTo() return Matcher<BaseStream<T, S>> #18

Open
facboy opened this issue May 30, 2019 · 3 comments · Fixed by mrwilson/java-8-matchers#7 · May be fixed by #19 or #22
Open

StreamMatchers.empty() and StreamMatchers.equalTo() return Matcher<BaseStream<T, S>> #18

facboy opened this issue May 30, 2019 · 3 comments · Fixed by mrwilson/java-8-matchers#7 · May be fixed by #19 or #22

Comments

@facboy
Copy link

facboy commented May 30, 2019

I think it would be better if StreamMatchers.empty() and StreamMatchers.equalTo() returned Matcher<S>, this would make them usable with a wider range of methods (in particular, methods that expect a Stream<T> argument).

@runeflobakk
Copy link
Contributor

I think this is a reasonable change, as one should rarely reference BaseStream in code. The interface is meant as base for extension, not to be referenced. There may be cases for very generic code to accept BaseStream as parameters, but very rarely as return type in an public API. BaseStream being a recursive type, are also difficult to correctly refer to.

I did some testing with a bit esoteric use of types, inferred through multiple methods, and found that the following does not compile with the proposed changes:

BaseStream<Character, Stream<Character>> expectedBaseStream = Stream.of('x', 'y', 'z');
assertThat("xyz", where(s -> s.chars().mapToObj(i -> (char) i), equalTo(expectedBaseStream)));

DescribableFunction<String, BaseStream<Character, Stream<Character>>> characters = s -> s.chars().mapToObj(i -> (char) i);
assertThat("xyz", where(characters, equalTo(Stream.of('x', 'y', 'z'))));

I still think this change makes sense, as the returned matcher should be as specifically typed as possible, and the matcher should be of the same type as the given argument. I.e. equalTo(Stream<T>) should create aMatcher<Stream<T>>.

The typing can be easily fixed by changing the signature of StreamMatchers.equalTo to

public static <T,S extends BaseStream<T, ? extends S>> Matcher<S> equalTo(S expected)

Adding the ? extends S to the signature allows the compiler to infer the common types for the arguments in the tests above.

@facboy For completeness, can you give a concrete example of code which fails to compile with the existing type signature of StreamMatchers.equalTo?

@runeflobakk runeflobakk linked a pull request Apr 11, 2020 that will close this issue
@facboy
Copy link
Author

facboy commented Apr 15, 2020

i haven't had time to look at this yet.

@runeflobakk
Copy link
Contributor

runeflobakk commented Apr 15, 2020

I think I have fixed this in #22
(I should have added the comment to your pull-request #19 instead of this issue, my bad)

runeflobakk pushed a commit to runeflobakk/java-8-matchers that referenced this issue Jul 29, 2020
Return type is parameterized to yield a Matcher of the specific
Stream-type, instead of just Matcher<BaseStream<_, _>>.
See unruly/java-8-matchers#18 for more
detailed discussion.
runeflobakk added a commit to runeflobakk/java-8-matchers that referenced this issue Jul 29, 2020
mrwilson pushed a commit to mrwilson/java-8-matchers that referenced this issue Aug 1, 2020
Return type is parameterized to yield a Matcher of the specific
Stream-type, instead of just Matcher<BaseStream<_, _>>.
See unruly/java-8-matchers#18 for more
detailed discussion.
mrwilson pushed a commit to mrwilson/java-8-matchers that referenced this issue Aug 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants