arashja / hamcrest

Automatically exported from code.google.com/p/hamcrest
0 stars 0 forks source link

generics too strict on IsEmptyCollection #97

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
current syntax:
assertThat(listOfIntegers, is(Matchers<Integer>.empty()));

proposed syntax:
assertThat(listOfIntegers, is(empty()));

snippet from the Matchers class:
  public static <E> org.hamcrest.Matcher<java.util.Collection<E>> empty() {
    return org.hamcrest.collection.IsEmptyCollection.<E>empty();
  }

the above code requires a Collection<E>, but the compiler can't infer E (no 
parameters are passed in to the method). This requires callers to specify E, 
like so:
  assertThat(listOfIntegers, is(Matchers<Integer>.empty()));

If IsEmptyCollection was non-generic, the problem should be solved, and 
really, it doesn't need to be generic since we don't care what it's a 
collection of, just so long as isEmpty() returns true.

Attached is an untested patch which should do the trick.

Original issue reported on code.google.com by bradcu...@gmail.com on 6 Aug 2009 at 10:32

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
correct patch

Original comment by bradcu...@gmail.com on 6 Aug 2009 at 10:44

Attachments:

GoogleCodeExporter commented 8 years ago
Here is another patch with a solution that uses generics.

Original comment by fr.rol...@gmail.com on 25 Sep 2009 at 9:04

Attachments:

GoogleCodeExporter commented 8 years ago
any takers on this? the patch supplied by @fr.roland seems like the one to go 
with

Original comment by bradcu...@gmail.com on 3 Nov 2009 at 5:39

GoogleCodeExporter commented 8 years ago
I agree, fr.roland's patch is the right one.

Original comment by joe.kear...@gtempaccount.com on 4 Nov 2009 at 9:17

GoogleCodeExporter commented 8 years ago
IsCollectionWithSize uses an even more elaborate return type for the factory 
method:

    public static <E> Matcher<? super Collection<? extends E>> hasSize(int size)

versus fr. roland's proposed:

    public static <E> Matcher<Collection<? extends E>> empty()

I believe there is no value in the ? super clause in IsCollectionWithSize's 
factory
method, but in any event, it appears both methods ought to have the same return 
type.

Original comment by ian.b.ro...@gmail.com on 12 Nov 2009 at 11:00

GoogleCodeExporter commented 8 years ago
ran into this issue today, found a post about it too:
http://marcphilipp.tumblr.com/post/393377359/generic-matcher-pitfalls

for now, I just created my own version of isEmpty

Original comment by lukewpat...@gmail.com on 23 Apr 2010 at 2:56

GoogleCodeExporter commented 8 years ago
Since the point of hamcrest is sugar and fluency, having to write 
Matchers<Integer>.empty() kinda misses the mark...  Can someone with committer 
rights please accept Roland's patch?

Original comment by eric.sir...@gmail.com on 12 May 2011 at 2:02

GoogleCodeExporter commented 8 years ago
Seconding the last comment.  It's coming up on 2 years.  Can someone commit 
this patch?

Original comment by stevegil...@gmail.com on 19 Jul 2011 at 7:49

GoogleCodeExporter commented 8 years ago
Hi, 

I ran into this ugly Matchers.empty() thing too the other day: 
is(not(Matchers<SomeObject>.empty())) is really not very friendly/readable...

I'va attached a new patch that, in addition to the changes made by fr.roland, 
includes modified unit tests for both concerned Matchers. 

I hope this will speed up the recognition/adoption of this issue.

Original comment by tom.goem...@gmail.com on 15 Nov 2011 at 12:40

Attachments:

GoogleCodeExporter commented 8 years ago
Dear committers, if this patch is not accepted, would you please provide 
feedback?

Original comment by cailiecr...@gmail.com on 15 Nov 2011 at 9:13

GoogleCodeExporter commented 8 years ago
It would really be nice if this gets fixed!

Original comment by jethroborsje on 21 Dec 2011 at 8:30

GoogleCodeExporter commented 8 years ago
I found this issue today for myself. It would be really nice if this two years 
old bug would be fixed before version 1.3 becomes final.
The last patch looks good for me.

Original comment by michael....@gmail.com on 13 Jan 2012 at 12:22

GoogleCodeExporter commented 8 years ago
Every time I would like to use "empty()" matcher I use "hasSize(0)" matcher and 
it is OK. Unfortunately it is less readable, but it works fine and is always 
better than:
assertThat(list.isEmpty(),equalTo(true))

Original comment by rafalmag on 17 Jan 2012 at 1:13

GoogleCodeExporter commented 8 years ago
just tagging as Java

Original comment by t.denley on 12 May 2012 at 10:42

GoogleCodeExporter commented 8 years ago
I'm new here, and I'm really sorry that no committers have even commented on 
this issue so far.  I'll do my best to hurry things along now.

Original comment by t.denley on 12 May 2012 at 1:47

GoogleCodeExporter commented 8 years ago
I've just committed what amounts to the tom.goemaes/fr.roland patches -- it 
tidied up the tests a little.  You'll see these changes in this commit on the 
new github reporistory for JavaHamcrest.  We hope to release a new version of 
hamcrest from here in the not-to-distant future.

https://github.com/hamcrest/JavaHamcrest/commit/7265db5fb6c95769499b599c6d25121d
72545594

If any of you are still out there and interested in this fix, please feel free 
to review the changes and try them out.  All feedback is welcome.

Original comment by t.denley on 12 May 2012 at 2:09

GoogleCodeExporter commented 8 years ago
Hey thanks a lot Tom!

Original comment by cailiecr...@gmail.com on 13 May 2012 at 6:49

GoogleCodeExporter commented 8 years ago
Hi, very nice changes. It works.
I just have written some acceptance tests, which does not compile before your 
fix.
See attachment.

Original comment by rafalmag on 13 May 2012 at 7:29

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for the feedback. Closing this as fixed, will be in the forthcoming 1.3 
release.

Original comment by t.denley on 14 May 2012 at 6:12