Coroutine suspend generated method naming scheme about vertx-lang-kotlin HOT 30 CLOSED

What about co…?

val id = vertx.coDeployVerticle(verticle);

This follows the convention of projects such as MockK.

from vertx-lang-kotlin.

@gagarski Overloads make it tough. The other option is to define a separate interface that contains only suspending versions of all the methods with their regular name. It makes sense for users of Kotlin coroutines since they would never need a version with a Handler parameter and if they want a void returning version (one that does not wait), then Kotlin coroutines provide a more explicit way to say that by using launch { vertx.deployVerticle(verticle) } to start a background activity and don't wait for it.

P.S. Ayway, I'd strongly recommend that all future-returning methods have their name ending with Async. Futures are very error-prone primitives and an additional attention to the fact that you are using something that returns a future is extremely helpful.

from vertx-lang-kotlin.

I'd use the same name for suspending functions. They would be distinguished on the call side by the absence of Handler parameter:

vertx.deployVerticle(verticle, Handler { ... }) 

// becomes

val id = vertx.deployVerticle(verticle)

from vertx-lang-kotlin.

@dualscyther if each method is implemented by just adding .await() to their Java version, we can probably inline all that code.
@chengenzhao the problem with using await as a prefix or async as suffix is that it makes these case really weird to read

async { awaitDeployVerticle(..) }.await()

// or

async { deployVerticleAsync(..) }.await()

from vertx-lang-kotlin.

So the problem is that Kotlin isn't the implementation language of Vertx, because otherwise it would most likely just be:

suspend fun deployVerticle(..): Int

This is what the Kotlin coroutines designers suggest when using suspending functions. In the case of returning a Deferred they suggest adding an Async suffix on the end like:

fun deployVerticleAsync(..): Deferred<Int>

However, this is suggested only if for whatever reason a suspend function can't be made instead. Await is a good suffix because it implies suspension (perhaps I'm influenced by the Kotlin Deferred.await method).

I think that fuzzy autocomplete is pretty good at finding these generated methods regardless of whether they are suffixed or prefixed. For something generic like awaitResult it makes sense to put it as a prefix since the user is concerned about just awaiting something generic. But for a specific method, I think it makes more sense to put it as a suffix because as was pointed out on Gitter by @gagarski:

"Also I think that suffix is better than prefix because with postfix you can see these wrappers among their non-suspend versions in code completion."

from vertx-lang-kotlin.

@elizarov It might work for some methods but not for all. deployVerticle already has an overload with one verticle argument. Also there are plans for Vertx 4.0 to add overloads that return a Vertx future (here) Unfortunately you cannot have suspend/non-suspend overload in Kotlin, so prefix/suffix is necessary.

from vertx-lang-kotlin.

@elizarov in case of vertx that is all non-blocking there is no ambiguity to return a future, everything is async. When a method is blocking however we suffix it with Blocking, e.g readFileBlocking()

The main change in vertx 4 is the usage of futures in addition of callbacks

from vertx-lang-kotlin.

BTW, if you actively using Vertx event bus with coroutines you will probably end up implementing something like this:

inline fun <T> EventBus.consumerSuspend(address: String, crossinline function: suspend (T) -> Unit)  =
        this.consumer(address) { msg ->
            launch(vertx.dispatcher()) {
                function(msg)
            }
        }

Otherwise you have to do launch in all your message handlers.

It would be nice to have it in Vertx out of the box (or generate such extensions if there are others like this).

from vertx-lang-kotlin.

@gagarski you can create also create a MessageConsumer that is a ReadStream that can be bound to a channel

from vertx-lang-kotlin.

Await suffix looks good to me. @vietj are you currently working on this feature? Do you not push your work in progress on some dev branch (couldn't find it in any of your 123 repos)? I would love to see where it is at as I was implementing something similar myself.

from vertx-lang-kotlin.

this feature @lfmunoz is actually in 3.6.0 snapshots

from vertx-lang-kotlin.

interesting ! I like it

what do other think ?

from vertx-lang-kotlin.

it seems also similar to the rx prefix we use for rxjava

from vertx-lang-kotlin.

@vietj do a +1 to @oripwk

from vertx-lang-kotlin.

done

from vertx-lang-kotlin.

I honestly prefer @elizarov suggestion, but I have no clue how much refactoring is needed compared to what is already available. What is the plan for 4.0? It would be nice to find a unique/similar solution that works well for both Coroutines and Reactive modules

from vertx-lang-kotlin.

@gmariotti I prefer his suggestion too. If it's possible to call the methods in the same name without adding any prefix or suffix is always preferable. But if it's too much refactor and the only way to go is to rename -- then the co prefix seems like a good alternative.

from vertx-lang-kotlin.

If rx is already used for the RxJava integration, then it definitely makes sense. It also would be interesting to do some kind of guide on how to migrate reactive code to coroutines code

from vertx-lang-kotlin.

I think the main issue would be ambiguity.

In addition in Vert.x 4, we will use a CompletionStage API which would collide with these methods.

from vertx-lang-kotlin.

kotlinx-coroutines-jdk8 offers extension methods for CompletionStage, it can probably help in avoiding collisions

from vertx-lang-kotlin.

in this case do we still need generation ?

the main interest of generation is to have more readable and fluent code, i.e we trade the idiom:

val something = awaitResult { foo.bar(it) }

for

val something = foo.coBar()

kotlinx-coroutines-jdk8 and CompletionStage would use a similar idiom with await I think (but better). So perhaps we won't generate coroutine in 4 ?

what do people think here ?

from vertx-lang-kotlin.

Indeed maybe in version 4 the code generation won't be necessary since you could do

val something = foo.bar().await()

which is itself very readable and elegant.

But for 3.6 please keep the code generation. We really need it :)

from vertx-lang-kotlin.

I honestly still think we should work into having suspending versions of each method, even if it means to have something like

suspend fun deployVerticle(verticle: AbstractVerticle) = deployVerticle(verticle).await()

but it really depends on how much refactoring is planned for Vert.x 4.

Have you started any work there @vietj?

from vertx-lang-kotlin.

suspend fun deployVerticle(verticle: AbstractVerticle) = deployVerticle(verticle).await()

We still cannot have suspend/non-suspend overload, so we still need a prefix/postfix.

from vertx-lang-kotlin.

@gagarski not if we go the interface route. Like a VertxK with all suspending methods and having extension methods to allow something like

Vertx.vertx().k.deployVerticle(..)

suspend val Vertx.k: VertxK

The main things that worries me of this solution is how difficult it is to implement and, more importantly, to maintain

from vertx-lang-kotlin.

If preffix is preferred, why not directly use await...?
awaitDeployVerticle();
eventbus.awaitSend();
since the basic idea is actually like
await methodName async() in other languages e.g. Dart/Flutter

void init(MyHomePageState state) async {
...
}

await init(state);

becomes
awaitInit(state);

from vertx-lang-kotlin.

co... looks like concurrently executing
but the basic idea is the vertx threads have to wait here for a while
in other languages, it is async/await pair
using Async suffix and await in front of those methods
so if we have to choose one of them
for prefix, it would be nice to use await, for suffix, async which is the idea of @elizarov

from vertx-lang-kotlin.

or maybe aw... instead of await...
if await... is too long to write
aw... would be similar to rx...

from vertx-lang-kotlin.

@chengenzhao The verbosity of await is not the issue, it's simply that it's more Kotlinic to make the regular name the suspend function.

@gmariotti This namespacing idea makes sense if we're able to implement it and with little runtime overhead.

from vertx-lang-kotlin.

@gmariotti This namespacing idea makes sense if we're able to implement it and with little runtime overhead.

+1

from vertx-lang-kotlin.