ACMNexus / google-collections

Automatically exported from code.google.com/p/google-collections
Apache License 2.0
0 stars 0 forks source link

Improve type signatures #169

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
I found a couple places that could make use of some changes to methods'
type signatures...

In com.google.common.collect.Sets, instead of:

    public static <E> SetView<E> intersection
    (final Set<E> set1, final Set<?> set2)
    //...

    public static <E> SetView<E> difference
    (final Set<E> set1, final Set<?> set2)
    //...

Use this:

    public static <E> SetView<E> intersection
    (final Set<? extends E> set1, final Set<? extends E> set2)
    //...

    public static <E> SetView<E> difference
    (final Set<? extends E> set1, final Set<? extends E> set2)
    //...

You'll need to make a couple obvious type adjustments to local variables,
and will also want to remove the apologetic notes in the associated javadoc
comments for those methods. Perhaps slightly non-obvious is this change to
com.google.common.collect.Iterators. Instead of:

    public static <T> UnmodifiableIterator<T> filter
    (final Iterator<T> unfiltered,
     final Predicate<? super T> predicate)

Use this:

    public static <T> UnmodifiableIterator<T> filter
    (final Iterator<? extends T> unfiltered,
     final Predicate<? super T> predicate)

Note: I just found these two examples in passing. There may be other places
that could use similar improvements.

Original issue reported on code.google.com by kind...@gmail.com on 14 May 2009 at 2:11

GoogleCodeExporter commented 8 years ago
I'm afraid I disagree with all three proposed changes.

First, the intersection of a Set<Number> and a Set<Integer> is logically a
Set<Integer>. With the current API, the user can get it as a Set<Integer>, by 
passing
the Set<Integer> as the first parameter. With your API, they can only get the 
result
as Set<Number>.  What is the benefit of your proposed signature? In particular, 
is
there any patently bogus code that it would prevent from compiling? (note that 
there
is no such thing as two sets that you can't compute the intersection of; at 
worst
that intersection is just empty.)

The same goes for difference:  a Set<Integer> minus a Set<Anything> is still 
always a
Set<Integer>, not a Set<Number> or Set<Object> as your change would force it to 
be.

Finally, if you filter an Iterator<Integer>, the result is an 
Iterator<Integer>. 
Your change allows the user to "pretend" that it's only an Iterator<Number> if 
they
want to, but this is a pointless feature for our API to support; they can cast 
or use
an adapter if they really need to (99% of the time, this is only because they 
are
interacting with a badly-written API, and it is not a design goal of our 
library to
ease working with badly-written APIs).

Please let me know if I have misunderstood any of your suggestions.

Original comment by kevin...@gmail.com on 28 May 2009 at 7:01

GoogleCodeExporter commented 8 years ago
I agree with your reasoning and retract my proposed changes. Thanks for the 
excellent
library.

Original comment by kind...@gmail.com on 28 May 2009 at 7:22