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.

Sign up for GitHub

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

Jump to bottom

Add successOrNothing and exceptionOrNull extensions #70

Merged
merged 4 commits into from
Nov 2, 2023
Merged

Add successOrNothing and exceptionOrNull extensions #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b942a89
Add successOrNothing and exceptionOrNull extensions
ZacSweers Nov 2, 2023
37ee725
Spotless
ZacSweers Nov 2, 2023
06a4f82
Use nothing
ZacSweers Nov 2, 2023
3123d79
Expanded doc
ZacSweers Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/eithernet.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,11 @@ public abstract interface annotation class com/slack/eithernet/ExperimentalEithe
}

public final class com/slack/eithernet/ExtensionsKt {
public static final fun exceptionOrNull (Lcom/slack/eithernet/ApiResult$Failure;)Ljava/lang/Throwable;
public static final fun fold (Lcom/slack/eithernet/ApiResult;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static final fun fold (Lcom/slack/eithernet/ApiResult;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static final fun successOrElse (Lcom/slack/eithernet/ApiResult;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static final fun successOrNothing (Lcom/slack/eithernet/ApiResult;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static final fun successOrNull (Lcom/slack/eithernet/ApiResult;)Ljava/lang/Object;
}

Expand Down
28 changes: 28 additions & 0 deletions src/main/java/com/slack/eithernet/Extensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,34 @@ public inline fun <T : Any, E : Any> ApiResult<T, E>.successOrElse(
is ApiResult.Failure -> defaultValue(this)
}

/**
* If [ApiResult.Success], returns the underlying [T] value. Otherwise, calls [body] with the
* failure, which can either throw an exception or return early (since this function is inline).
*/
public inline fun <T : Any, E : Any> ApiResult<T, E>.successOrNothing(
body: (ApiResult.Failure<E>) -> Nothing
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of Nothing here implies that body will never return (ie. it will throw) but there's no way to ensure that happens. Returning Unit would seem appropriate, but renaming the method to successOrUnit doesn't have the same ring

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of Nothing ensures that the body either throws or returns: https://www.zacsweers.dev/its-nothing/

Using Unit would actually not accomplish the goal of this function :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combining Nothing with an inline function and non-local returns is a cool pattern 👍 .

I wonder if we should document this somehow. Like me, I expect many/most will probably have to refresh their memory about what exactly Nothing is for and come away with only half the story

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the type system not help make that clear here? If you tried to just use Unit or anything other than throw/return, this would fail to compile.

Copy link

@kierse kierse Nov 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I wasn't clear. I'm not arguing for using Unit or any other return type. I'm suggesting that someone unfamiliar with Nothing will read the docs and see that they are expected to throw and stop all investigation there. They are likely to overlook the fact that a non-local return (made possible by the use of an inline function) is also a valid option.

I was trying to suggest that we document this a bit better to expose that supported usage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have a suggestion? Nothing is a pretty standard language feature of Kotlin so I'm not sure what exactly to say other than refer to its doc

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not clear we could expand the doc to be along the line of;
Otherwise, calls [body] with the failure, which should throw or return.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works! Thank you both

): T =
when (this) {
is ApiResult.Success -> value
is ApiResult.Failure -> body(this)
}

/**
* Returns the encapsulated [Throwable] exception of this failure type if one is available or null
* if none are available.
*
* Note that if this is [ApiResult.Failure.HttpFailure] or [ApiResult.Failure.ApiFailure], the
* `error` property will be returned IFF it's a [Throwable].
*/
public fun <E : Any> ApiResult.Failure<E>.exceptionOrNull(): Throwable? {
return when (this) {
is ApiResult.Failure.NetworkFailure -> error
is ApiResult.Failure.UnknownFailure -> error
is ApiResult.Failure.HttpFailure -> error as? Throwable?
is ApiResult.Failure.ApiFailure -> error as? Throwable?
}
}

/** Transforms an [ApiResult] into a [C] value. */
public fun <T : Any, E : Any, C> ApiResult<T, E>.fold(
onSuccess: (ApiResult.Success<T>) -> C,
Expand Down
50 changes: 50 additions & 0 deletions src/test/kotlin/com/slack/eithernet/ExtensionsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.slack.eithernet

import com.google.common.truth.Truth.assertThat
import kotlin.test.fail
import okio.IOException
import org.junit.Test

Expand Down Expand Up @@ -129,4 +130,53 @@ class ExtensionsTest {
)
assertThat(folded).isEqualTo("Failure")
}

@Test
fun successOrNothingShouldEscape() {
val result: ApiResult<*, *> = ApiResult.networkFailure(IOException())
result.successOrNothing {
return
}
fail("Should not reach here")
}

@Test
fun exceptionOrNull_with_networkFailure() {
val exception = IOException()
val result = ApiResult.networkFailure(exception)
assertThat(result.exceptionOrNull()).isSameInstanceAs(exception)
}

@Test
fun exceptionOrNull_with_unknownFailure() {
val exception = IOException()
val result = ApiResult.unknownFailure(exception)
assertThat(result.exceptionOrNull()).isSameInstanceAs(exception)
}

@Test
fun exceptionOrNull_with_throwableApiFailure() {
val exception = IOException()
val result = ApiResult.apiFailure(exception)
assertThat(result.exceptionOrNull()).isSameInstanceAs(exception)
}

@Test
fun exceptionOrNull_with_throwableHttpFailure() {
val exception = IOException()
val result = ApiResult.httpFailure(404, exception)
assertThat(result.exceptionOrNull()).isSameInstanceAs(exception)
}

@Test
fun exceptionOrNull_with_nonThrowableApiFailure() {
val result = ApiResult.apiFailure("nope")
assertThat(result.exceptionOrNull()).isNull()
}

@Test
fun exceptionOrNull_with_nonThrowableHttpFailure() {
val result = ApiResult.httpFailure(404, "nope")
assertThat(result.exceptionOrNull()).isNull()
}
}