search

unruly / java-8-matchers

Hamcrest Matchers for Java 8 features
MIT License
23 stars 5 forks source link

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

Open facboy opened 5 years ago

facboy commented 5 years ago

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 commented 4 years ago

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?

facboy commented 4 years ago

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

runeflobakk commented 4 years ago

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)