2
\$\begingroup\$

So, here's a task. A background thread may or may not call all of it's listeners some time after sendRequest() was called (in other words sendRequest() schedules a possible event). The required function must:

  1. Subscribe to those events
  2. Call sendRequest() periodically until the function returns anything
  3. Return the first suitable result (filtering is done in the listener and is omitted in this snippet) or null after a timeout
  4. Unsubscribe from those events regardless of the result
suspend fun findServer(): Server? {
    var listener: ((Server) -> Unit)? = null
    return try {
        withTimeoutOrNull(10000) {
            suspendCancellableCoroutine<Server> { continuation ->
                listener = { server: Server ->
                    continuation.resume(server)
                }.also { discoveryService.addListener(it) }

                launch {
                    while (continuation.isActive) {
                        discoveryService.sendRequest()
                        delay(1000)
                    }
                }
            }
        }
    } finally {
        listener?.let { discoveryService.removeListener(it) }
    }
}

The current implementation seems to work fine, but the nullable listener variable feels kind of clunky and requires using also to keep the listener reference, which in turn breaks type inference and makes me specify listener's parameter type (no big deal, but...). I could use a lateinit var here, but it probably can lead to exceptions when withTimeoutOrNull() stops before the listener was initialized.

Any recommendations on improving this snippet? Or am I, in fact, doing everything totally wrong here?

\$\endgroup\$
1
\$\begingroup\$

Think it's pretty good and you can't think of much else. Some tiny bits I can think of:

  • Make discoveryService parameter.
  • Do not use launch without specifying CoroutineScope.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Well, the application is built on dependency injection and discoveryService is an injected component so it shouldn't be a parameter. I'm not too comfortable with CoroutineScopes yet, but it seems that withTimeoutOrNull provides a scope. Should I wrap it somewhere? \$\endgroup\$ – sibwaf Dec 10 '19 at 14:45
  • \$\begingroup\$ Yeah I get it and I think you are right and withTimeoutOrNull indeed provides scope, my bad :) \$\endgroup\$ – K.H. Dec 10 '19 at 16:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.