Зачем использовать назначение в сравнении?

Читая исходный код, я наткнулся на этот метод в источниках JDK. Обратите внимание на объявление и инициализацию v и newValue. У нас есть "хорошие" неопределенные значения, присваивание в сравнениях, что "здорово", и дополнительные скобки для худшей читаемости. И другой код пахнет.

default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction) {
    Objects.requireNonNull(mappingFunction);
    V v;
    if ((v = get(key)) == null) {
        V newValue;
        if ((newValue = mappingFunction.apply(key)) != null) {
            put(key, newValue);
            return newValue;
        }
    }

    return v;
}

Но почему? Есть ли реальная выгода в написании кода вышеописанным способом, а не простым (в идеале, с отрицательным v сравнением):

default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction) {
    Objects.requireNonNull(mappingFunction);
    V v  = get(key);
    if (v == null) {
        V newValue = mappingFunction.apply(key);
        if (newValue != null) {
            put(key, newValue);
            return newValue;
        }
    }

    return v;
}

Есть ли какая-то реальная выгода, о которой я не знаю (помимо демонстрации Java-конструкций), вместо того, чтобы идти "простым" способом?

Ответы

Ответ 1

#microoptimization (но в случае стандартной библиотеки это может иметь значение) и:

#inertia: этот шаблон был распространен среди программистов на C еще в 90-х годах, поэтому титаны компьютерных наук все еще могут использовать этот стиль.

Нет смысла писать такой код для новой бизнес-логики, если только производительность не критична.

(Микро) оптимизация:

Байт-код, созданный javac (JDK 11) для исходной ("плохой") версии, на одну JVM-операцию меньше, чем (более хороший) код. Зачем? Версия JDK "использует" возвращаемое значение оператора присваивания (вместо загрузки значения из переменной) для оценки условия if.

Тем не менее, это скорее ограничение возможностей оптимизации javac чем причина написания менее читаемого кода.

Вот байт-код для версии JDK, процитированный в вопросе:

   0: aload_2
   1: invokestatic  #2                  // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
   4: pop
   5: aload_0
   6: aload_1
   7: invokevirtual #3                  // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
  10: dup
  11: astore_3
  12: ifnonnull     39
  15: aload_2
  16: aload_1
  17: invokeinterface #4,  2            // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
  22: dup
  23: astore        4
  25: ifnull        39
  28: aload_0
  29: aload_1
  30: aload         4
  32: invokevirtual #5                  // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
  35: pop
  36: aload         4
  38: areturn
  39: aload_3
  40: areturn

Ниже приведен байт-код более читаемой версии:

public V computeIfAbsent(K key,
                         Function<? super K, ? extends V> mappingFunction) {
    Objects.requireNonNull(mappingFunction);
    final V v = get(key);
    if (v == null) {
        final V newValue = mappingFunction.apply(key);
        if (newValue != null) {
            put(key, newValue);
            return newValue;
        }
    }

    return v;
}

.. и байт-код:

   0: aload_2
   1: invokestatic  #2                  // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
   4: pop
   5: aload_0
   6: aload_1
   7: invokevirtual #3                  // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
  10: astore_3
  11: aload_3
  12: ifnonnull     40
  15: aload_2
  16: aload_1
  17: invokeinterface #4,  2            // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
  22: astore        4
  24: aload         4
  26: ifnull        40
  29: aload_0
  30: aload_1
  31: aload         4
  33: invokevirtual #5                  // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
  36: pop
  37: aload         4
  39: areturn
  40: aload_3
  41: areturn

Ответ 2

Я думаю, что это в основном вопрос предпочтений; У меня сложилось впечатление, что Даг Ли предпочитает этот лаконичный стиль. Но если вы посмотрите на оригинальную версию метода, это немного более разумно, так как он связан с newValue который требует встроенного присваивания:

default V computeIfAbsent(K key,
        Function<? super K, ? extends V> mappingFunction) {
    V v, newValue;
    return ((v = get(key)) == null &&
            (newValue = mappingFunction.apply(key)) != null &&
            (v = putIfAbsent(key, newValue)) == null) ? newValue : v;
}

Ответ 3

Я согласен: это запах кода, и я определенно предпочитаю вторую версию.

Тайное использование присваивания и сравнения в одном выражении может привести к несколько более короткому коду, если у вас есть цикл, например:

V v;
while ((v = getNext()) != null) {
    // ... do something with v ...
}

Чтобы удалить запах кода, вам нужно больше кода, и вам нужно назначить переменную в двух местах:

V v = getNext();
while (v != null) {
    // ...
    v = getNext();
}

Или вам нужно переместить условие выхода из цикла после присваивания:

while (true) {
    V v = getNext();
    if (v == null)
        break;
    // ...
}

Для утверждения if это, конечно, не имеет смысла. И даже для циклы я бы этого избегал.