The CompletableDeferred having strange constructors

[tekener]

We spend hours last week to find a bug introduced by code like this:

fun getValueAsync(someCondition: Boolean): Deferred<Int?> = runBlocking {
  if(someCondition){
   CompletableDeferred(5)
  } else {
   CompletableDeferred(null)
 }
}

What i thought i am doing is returning a Deferred resolving to null.
But there are two constructors, one with T and one with parent: Job?. If you call it like CompletableDeferred(5) it resolved to 5, if you call it like CompletableDeferred(null) it is never resolving because the null resulting in calling the constructor with the parent Job.

I would suggest to get rid of the constructors and provide some static methods with meaningful names instead (like in scala CompletableDeferred.successful(null)

kotlinx.coroutines/kotlinx-coroutines-core/common/src/CompletableDeferred.kt

Line 68 in 5a71f7b

public fun <T> CompletableDeferred(parent: Job? = null): CompletableDeferred<T> = CompletableDeferredImpl(parent)

kotlinx.coroutines/kotlinx-coroutines-core/common/src/CompletableDeferred.kt

Line 74 in 5a71f7b

public fun <T> CompletableDeferred(value: T): CompletableDeferred<T> = CompletableDeferredImpl<T>(null).apply { complete(value) }

[qwwdfsad]

As a side note, the first overload wins because it's more specific: any argument supplied to the first overload can also be supplied to the second, which makes the first one more specific. Inferred via return type generic argument is not used during such substitution check.

The problem is indeed a bummer and we either can do something with a less popular overload (Job?) or introduce an IDE inspection for that (e.g. "consider using an explicit parameter name")

[merkinth]

As a workaround you could cast null to the expected type, like CompletableDeferred(null as? Int)

[elizarov]

Yes, the overload of the CompletableDeferred constructor is an unfortunate historical accident. If we have an alternative API for creating parent/child hierarchies in the future, we should definitely consider deprecating and hiding the constructor with Job?.

Another road is, indeed, creating explicitly named "successful/completed" factory functions. Also, we can declare yet another overload specifically for CompletableDeferred(null) pattern with the following signature:

fun <T> CompletableDeferred(value: Nothing?): CompletableDeferred<T?>

It can also be marked as deprecated to force anyone who wrote CompletableDefferred(null) into thinking mode at what exactly they wanted to achieve.