-
Notifications
You must be signed in to change notification settings - Fork 60
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
V5.0 #34
Comments
Thank you for creating this issue. We'll use it as the place to plan new features for a v5.0 release. Switch to sealed interfacesKotlin 1.5 brings sealed interfaces support. Using Move away from nested sub-classesMoving the sub classes of sealed interface NetworkResponse<S, E>
data class Success<S, E>(val data: S?) : NetworkResponse<S, E>
sealed interface Error<S, E> : NetworkResponse<S, E>
data class ServerError<S, E>(val error: E?) : Error<S, E>
data class NetworkError<S, E>(val error: E?) : Error<S, E>
data class UnknownError<S, E>(val error: E?) : Error<S, E>
when (response) {
is Success ->
is NetworkError ->
is ServerError ->
is UnknownError ->
} Improve generic parameter handlingSome sub-types of I'll update this issue as we progress towards 5.0. Any other feature requests or bug reports are very welcome. |
Perfect.
I would also love to see a `completable` state i.e success with unit but
without needing to specify it as `Response<Unit>`
Maybe something like a `Completable` type alias.
…On Sun, May 9, 2021, 11:43 Kshitij Chauhan ***@***.***> wrote:
Thank you for creating this issue. We'll use it as the place to plan new
features for a v5.0 release.
Switch to sealed interfaces
Kotlin 1.5 brings sealed interfaces
<https://kotlinlang.org/docs/sealed-classes.html> support. Using
NetworkResponse with sealed interfaces would allow greater flexibility in
dividing the Success and Error types. The current Error interface in the
library is not meant to be extended, but client applications are free to do
so. Sealed interfaces would eliminate this possibility.
Move away from nested sub-classes
Moving the sub classes of NetworkResponse to top level classes would
reduce verbosity when checking their types in a when expression:
sealed interface NetworkResponse<S, E>data class Success<S, E>(val data: S?) : NetworkResponse<S, E>
sealed interface Error<S, E> : NetworkResponse<S, E>data class ServerError<S, E>(val error: E?) : Error<S, E>data class NetworkError<S, E>(val error: E?) : Error<S, E>data class UnknownError<S, E>(val error: E?) : Error<S, E>
when (response) {
is Success ->
is NetworkError ->
is ServerError ->
is UnknownError ->
}
Improve generic parameter handling
Some sub-types of NetworkResponse utilize Nothing as a generic type
parameter, which is semantically unsound. Nothing is reserved for
extra-ordinary situations that are never meant to happen, such as
exceptions. Similarly, the case of empty server bodies is handled with
Unit. v5.0 should make improvements to this API.
------------------------------
I'll update this issue as we progress towards 5.0. Any other feature
requests or bug reports are very welcome.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#34 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD563XHL7PDZA4AZDPVXNRDTMZDLLANCNFSM4333GS3Q>
.
|
I've also raised an issue related to 204 responses, which are success response which have no body. I noticed we check if the body is not null to determine whether we have a success or a server error response. Maybe we need another success representation without a body, which hopefully could be solved with sealed interfaces as well. |
Looking forward to the v5 release. When Kotlin 1.7 enforces exhaustive when statements, the sealed interfaces change will make a nice difference. |
I've been working on v5.0 for the past couple of weekends. I might be able to provide you with an update before the holiday season starts, but no promises :) |
I opened an issue so we can discuss what changes can and or should be make in v5.0.
edit:
I wanted to label the “issue” but I can't
The text was updated successfully, but these errors were encountered: