maidh91 / guava-libraries

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

`Ordering` should provide a `lexicographical`-like method that returns `Ordering<Iterator<? extends S>>` #380

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
(Obviously, one can't change `lexicographical` itself without breaking backward 
compatibility, so, a new method should be made going forward that returns 
`Ordering<Iterator<? extends S>>` instead of `Ordering<Iterator<S>>`.)

== Context ==

From `Ordering.java`:

{{{
    /*
     * Note that technically the returned ordering should be capable of
     * handling not just {@code Iterable<S>} instances, but also any {@code
     * Iterable<? extends S>}. However, the need for this comes up so rarely
     * that it doesn't justify making everyone else deal with the very ugly
     * wildcard.
     */
}}}

The `<? extends S>` use case may be "so rare", but I think it should be less 
rare.

== A use case for `<? extends S>` ==

Using `<? extends S>` is quite useful for read-only container types, when used 
in tandem with `ImmutableList` and the like.

The idea behind the `Immutable*` classes is that operations that change the 
collection are rejected at runtime. This is because Java doesn't have `const`. 
However, there is a simple way to emulate it (to a degree), and that is by 
using using `ImmutableList<? extends Whatever>`.

This disables operations like `add`, `set`, etc., that take an object of type 
`E` (the type parameter of the list), while still leaving operations like 
`contains`, `indexOf`, and other methods that take `Object` still working.

This doesn't catch all operations that change the list (`remove` takes 
`Object`, not `E`), but it does catch enough common ones to be a useful 
technique (in my book at least).

And it's a technique that should be encouraged, by having `Ordering` support 
the use of such collections in its lexicographical comparator. As mentioned 
earlier, it's infeasible to change `lexicographical` itself, but, a second 
method that does support it should be provided.

Original issue reported on code.google.com by cky944 on 16 Jul 2010 at 5:45

GoogleCodeExporter commented 9 years ago
I hear and understand, but can't agree.

Note that ImmutableList<? extends Whatever> doesn't prevent you from calling 
.add(null) either.  I don't personally think it's a good idea to do this.  
Wildcards are things we using, in method parameters, because we have to, but I 
really don't like to see them any more than necessary -- they're way too bulky 
and unreadable.  The notion of using them only on parameters but never on 
return values begins with the JDK and we followed it.

The justification is weak, and it would have to be _very_ strong for us to want 
to put two methods in an API that do the exact same thing but for a tweak to a 
type.  (For example, the case for ListMultimap.asListMap() was stronger than 
this but still not strong enough.)

Sometimes you have an ImmutableList<? extends Whatever> whether you wanted to 
or not (say, because it was passed to you in a public API).  Your workaround is 
to throw away the wildcard, at least just for the purpose of using 
Ordering.lex().

    ImmutableList<Whatever> tmp = ImmutableList.copyOf(list);

As you probably know, this doesn't really copy anything.

Original comment by kevinb@google.com on 17 Jul 2010 at 4:09

GoogleCodeExporter commented 9 years ago
The workaround you mentioned is perfectly fine for current purposes. It may be 
worth documenting the workaround in a code comment, probably accompanying the 
one pasted in the original post. That way, people who are adamant about using 
wildcards have an "official" way out, without cluttering up the doc comments 
for "most people".

Thanks for the explanation, and yes, totally understand about not having two 
methods that really do the same thing. As regards the "no wildcards in return 
types", yes, that is explained in Effective Java 2nd ed, but, my understanding 
of it is that it applies shallowly, i.e., only to the top-level type arguments. 
Perhaps I misread the book.

Also, thanks for articulating your difference of opinion on the use of 
wildcards. At least with the workaround, we can effectively each do our own 
thing without getting in the other's way. :-)

Original comment by cky944 on 17 Jul 2010 at 5:46

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:15

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:09