Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic type bounds of IsIterableContaining are inconsistent with those of IsMapContaining #252

Closed
nibsi opened this issue Mar 22, 2019 · 4 comments

Comments

@nibsi
Copy link

nibsi commented Mar 22, 2019

Hello all! Thanks a lot for the new Hamcrest 2.1 library, it's a pleasure to work with and I can also use it easily in my Java 9 modules.

The IsIterableContaining matcher extends TypeSafeDiagnosingMatcher<Iterable<? super T>>. I understand that the goal is to, for instance, match a Set<Number> when it has an Integer.

Why then does IsMapContaining extend TypeSafeMatcher<Map<? extends K, ? extends V>>? Wouldn't it be just as reasonable to match a Map<Object, Object> when it has a String key? Or a Long value?

@tumbarumba
Copy link
Member

Writing library code with Java generics is a bit like (╯ರ ~ ರ)╯︵ ┻━┻

Perhaps @sf105 might have some insights as to how the current implementation came to be the way it is?

@nibsi
Copy link
Author

nibsi commented Apr 3, 2019

Collection<Banana> bananas = singleton(banana);
Collection<Fruit>  fruits  = singleton(banana);
    
assertThat(fruits,  hasItem(banana));
assertThat(bananas, hasItem(fruit) );  // this one is currently invalid

I think this makes sense: Why would I want to check if a collection of some specific type contains some less specific type? Usually I already know the actual runtime type of the value I use to create the matcher with, because I create it myself, whereas the collection being examined is passed in and could be anything.

The same is true for maps. Therefore I would propose to make the hasKey() and hasValue() matchers contravariant.

@foal
Copy link

foal commented Sep 5, 2024

The IsIterableContaining matcher extends TypeSafeDiagnosingMatcher<Iterable<? super T>>.

The IsIterableContaining have to extend TypeSafeDiagnosingMatcher<Iterable<? extends T>>.
Usualy you have a collection of entities (e.g. Car). This list can contain any object that extends Car (List<? extent Car>). And you want to apply to each element the Matcher that can check the Car or it's parent (Matcher<? super Car>).

So, you want to construct Matcher that take the Iterable<? extends Car> and apply to it given Matcher<? super Car>

@tumbarumba
Copy link
Member

I've been reading up on poloymorphic collections in Java, and the PECS rule (producer extends, consumer super). I've created #422 as an attempt to address this issue. There's a new test in IsIterableContainingTest that demonstrates the problem and the fix.

I'm a bit worried that this change might impact a lot of existing code. I'd appreciate if folks could check out that fork and give it a test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants