Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create abstract methods instead of open methods #90

Open
Globegitter opened this issue Nov 20, 2019 · 3 comments
Open

Create abstract methods instead of open methods #90

Globegitter opened this issue Nov 20, 2019 · 3 comments

Comments

@Globegitter
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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")
    }
``

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants