MedwynLiang / google-collections

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

static method Functions.toStringFunction() should be parametrized #301

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The static function 
com.google.common.base.Function.Function.toStringFunction()
is not parametrized and needs casting when used, leading to "unchecked" 
warnings.

It should perform the casting internally, ala parametrized method:
  org.java.util.Collections.emptyList().

A possible impl would be:

  /**
   * Returns a function that calls {@code toString()} on its argument. The
   * function does not accept nulls; it will throw a
   * {@link NullPointerException} when applied to {@code null}.
   */
  @SuppressWarnings("unchecked")
  public static <T> Function<T, String> toStringFunction() {
    return (Function<T, String>)ToStringFunction.INSTANCE;
  }

Original issue reported on code.google.com by ankos...@gmail.com on 23 Nov 2009 at 9:04

GoogleCodeExporter commented 9 years ago
(aargh...)
version reported 1.0-RC4

Original comment by ankos...@gmail.com on 23 Nov 2009 at 9:05

GoogleCodeExporter commented 9 years ago
Logically, you can do anything with a Function<Object, String> that you could 
with a 
Function<T, String>. The likely reason for your difficulty is that you're 
passing 
this function into a method from another library that did not declare the 
appropriate 
wildcards, as explained thoroughly in Effective Java 2e Item 28.

Adding a parameter to toStringFunction() is equivalent to saying, "I know you 
can 
really work on any Object, but could you please pretend that you only work for 
Foos?"   
It doesn't make much sense.

Original comment by kevin...@gmail.com on 23 Nov 2009 at 9:21

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 23 Nov 2009 at 9:22

GoogleCodeExporter commented 9 years ago
Incidentally, we got this wrong for a few of the methods in the Predicates 
class 
(alwaysFalse, alwaysTrue, isNull, notNull).  As a result, we've ended up with 
388 
cases within Google (so far) where the user was forced to specify an explicit 
type 
parameter, as in "Predicates.<String>alwaysTrue()", which is ugly and senseless.

Original comment by kevin...@gmail.com on 23 Nov 2009 at 9:24

GoogleCodeExporter commented 9 years ago
> The likely reason for your difficulty is that you're passing 
> this function into a method from another library that did not 
> declare the appropriate wildcards, ...

Yet, within the sources of these predicate cases, you are forced to declare the 
suppress-warnings annotation yourself, for instance: 
  @GwtCompatible(serializable = true)
  @SuppressWarnings("unchecked")
  public static <T> Predicate<T> alwaysTrue() {
    return (Predicate<T>) AlwaysTruePredicate.INSTANCE;
  }

Besides, that is the exact reason behind the decision to declare 
Functions.toStringFunc() as a _method_ instead of a _field_, 
as demonstrated by the quite-analogous JDK's pair: 
  Collections.EMPTY_LIST <==> Collections.emptyList() 

Also, i cannot understand what do you mean by this: 
  «388 cases within google...»?
It may be more code to write, but it's the correct code.
It isolates the "suppress" annotation only within lib-code, 
instead of letting it proliferate throughout user code.

Original comment by ankos...@gmail.com on 24 Nov 2009 at 12:00