search

marcoferrer / kroto-plus

gRPC Kotlin Coroutines, Protobuf DSL, Scripting for Protoc
Apache License 2.0
494 stars 28 forks source link

Create abstract methods instead of open methods #90

Open Globegitter opened 4 years ago

Globegitter commented 4 years ago

How come the abstract NameImplBase class generates open functions rather than abstract functions? I think it would be nice to get a compile time error if a method has not been implemented, rather than a runtime error.

marcoferrer commented 4 years ago

The choice came from trying to expose an API that mimicked the existing grpc-java behavior as close as possible. Generating open methods over abstract methods adheres to the principal of least surprise. Another reason is that it is common to evolve the api contract separately from the implementation. Most orgs Im familiar with will have a separate review process for their proto definitions.

But to your point, some users might find it beneficial to enforce method implementation at compile time. This could be hidden behind a configuration flag to make it opt-in only. Im interested in hearing what the thoughts are about this feature from other users in the community.

Globegitter commented 4 years ago

Thanks again for the quick response - that makes sense then and I can see the reasoning why it would not be default. If it is possible to be supported that would be great, but if no one else sees any value in this that is also fair.

Globegitter commented 4 years ago

One thing that is possible if a new rpc method is added but one does not want to implement the method one could just add it with a TODO, as below, leading to a very similar effect as when not overriden:


override suspend fun rpcFn(request: MyRequest): MyResponse {
        TODO("not implemented")
    }
``