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

[REQ] [KOTLIN] Don't use TODO() function provided by Kotlin #3529

Closed
toXel opened this issue Aug 1, 2019 · 4 comments
Closed

[REQ] [KOTLIN] Don't use TODO() function provided by Kotlin #3529

toXel opened this issue Aug 1, 2019 · 4 comments

Comments

@toXel
Copy link

toXel commented Aug 1, 2019

The use of the Kotlin function TODO() can IMO be dangerous sometimes. For example here:

return when(mediaType) {
JsonMediaType -> Serializer.moshi.adapter(T::class.java).fromJson(bodyContent)
else -> TODO("responseBody currently only supports JSON body.")
}

When mediaType is not application/json the app or program, in which the client is used, just crashes. AFAIK there is no way to catch these exceptions without modifying the ApiClient class.

Describe the solution you'd like

Replace TODO() calls with catchable exceptions or return null and add an // TODO comment. I've tested both attempts and they worked without crashing the app.

@jimschubert
Copy link
Member

👍 I think the custom exception option would be the better approach than an implementer comment and returning null.

@jimschubert
Copy link
Member

I opened #3611 to change TODO() to throw UnsupportedOperationException. What are your thoughts on this exception type rather than some custom exception?

@toXel
Copy link
Author

toXel commented Aug 12, 2019

I think that's a great solution. Thank you for the PR! I hope it'll be merged soon.

@jimschubert
Copy link
Member

I've merged the PR. Please keep an eye out for the change in the latest snapshot.

Feel free to reopen the issue if the fix needs any adjustments.

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

No branches or pull requests

2 participants