ben-manes / caffeine

A high performance caching library for Java
Apache License 2.0
15.64k stars 1.58k forks source link

Kind of wish get's type parameter was slightly different #1691

Closed sixcorners closed 3 months ago

sixcorners commented 3 months ago

I kind of wish get had a slightly different signature.

  @PolyNull
  <T extends K> V get(T key, Function<T, ? extends @PolyNull V> mappingFunction);

instead of

  @PolyNull
  V get(K key, Function<? super K, ? extends @PolyNull V> mappingFunction);

that way the parameter to the function on the other side was exactly the type I was passing to the first parameter of get. So if my cache used Contextual<?> as the key and I was trying to call get with Contextual<T> I would get a Contextual<T> inside the mapping function instead of Contextual<?>.

That said the difference is kind of small to me. If it mattered a whole lot I could just capture the value I'm using as the key in the mappingFunction lambda and I kind of ended up having to use raw types elsewhere to make everything work so the type of the parameter that gets used here wouldn't be a big improvement to my code.

Also I don't know all of the consequences of changing something like this and I don't know if it would work for getAll.

ben-manes commented 3 months ago

fwiw, the current signature matches Map.computeIfAbsent. I believe the difference is that this form allows a wider range of functions, e.g. an Integer key can use a function that accepts a Number argument, e.g. boolean isPositive(Number). In your desired signature, it would be restricted to subtypes of K so it provides a more concrete guarantees for the type.

Other than method references, the benefit of the Function type with a key parameter instead of a Supplier is that you can use a non-capturing lambda. That avoids an allocation as the JVM can cache it, but since its young/eden space the cost is minimal. So as you said, you could just ignore the function's key param and have the function use captured key instead.

sixcorners commented 3 months ago

Hmm.. Dang... Making this kind of change to computeIfAbsent would end up breaking all subclasses. I guess I can't really ask them to make a change like this now.

I think you would be able to use a function that accepts a Number even if the receiver of the lambda is expecting an Integer accepting function.

    static boolean isPositive(Number n) {return true;}
...
    Function<Integer, Boolean> x = Test::isPositive;

This change would make it so that you could use a boolean isPositive(int n) even if the Cache was using Number as it's K.

idk. anyway. I'm fine leaving it like it is since the benefit is tiny and the backwards compatibility might be a problem. thanks for the consideration.

ben-manes commented 3 months ago

I think it works? I'm a little surprised to be honest. but as you mentioned, I think its minor enough to leave as is and I'm unsure about compatibility. Good question tho!

  void test() {
    var c = new C<AtomicInteger, Boolean>();
    var key = new AtomicInteger(5);
    boolean value = c.get(key, this::isPositive);
    boolean value2 = c.get(new X(), this::isNegative);
  }

  class X extends AtomicInteger {}
  class C<K, V> {
    <T extends K> V get(T key, Function<T, ? extends V> mappingFunction) {
      return null;
    }    
  }

  boolean isPositive(Number n) {
    return true;
  }

  boolean isNegative(X x) {
    return false;
  }
ben-manes commented 3 months ago

ChatGPT explains this is an implementation quirk of method references. If you switch to an explicit lambda syntax it won't work, because then it is stricter about the types. The method reference behaves as a shorthand for o -> isPositive(o) so it works with the super type. I didn't know that difference about this difference, always some corner of Java to learn.


Method references and lambdas in Java serve similar purposes—they both represent functional interfaces and can be used as arguments for methods or assigned to functional interface variables. However, they differ in how they capture the context and how type inference is applied.

Method References:

  • Method references provide a shorthand syntax for referring to methods by name. They can refer to static methods, instance methods, constructors, or instance methods of existing objects.
  • Method references implicitly capture their target types based on the context in which they are used. The compiler infers the target type based on the signature of the functional interface to which the method reference is being assigned.
  • When a method reference is used, the compiler attempts to match the signature of the referenced method with the functional interface's abstract method. If the method reference's signature matches the expected signature of the functional interface, it's considered a valid method reference.

Lambdas:

  • Lambdas are anonymous functions that can be used to implement functional interfaces. They provide a concise syntax for defining small, one-off functions inline.
  • Lambdas explicitly declare their parameter types (if needed) and provide the implementation of the functional interface's abstract method within the lambda body.
  • Type inference for lambdas is also based on the context, but it's more explicit because the parameter types and return type (if not inferred from the context) are explicitly declared within the lambda expression itself.

In the context of the original question, the difference arises because method references rely on the compiler to infer the types based on the context, sometimes leading to unexpected behavior if the method reference's signature doesn't strictly match the expected signature of the functional interface. Lambdas, on the other hand, explicitly declare their parameter types, making the type inference more transparent and less prone to ambiguity.

cpovirk commented 3 months ago

I wonder if a hybrid of the two couple work, preserving the ? super but still adding the type parameter:

  @PolyNull
  <T extends K> V get(T key, Function<? super T, ? extends @PolyNull V> mappingFunction);

I doubt it's worth pursuing (given even the small chance of compatibility problems), but it seems in principle like it might work?