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

Add tryRecoverIf and tryRecoverUnless functions #94

Closed

Conversation

Jhabkin
Copy link
Contributor

@Jhabkin Jhabkin commented Jan 9, 2024

Sometimes there is a need to recover certain errors and this recovery may produce an error on its own. Existing recoverIf function doesn't fit this case because the result of transformation is wrapped in Ok. Added tryRecoverIf and tryRecoverUnless for the described case.

@michaelbull
Copy link
Owner

Could you give an example use-case where this would be used?

The naming is a little misleading currently, a function prefixed with try would imply to me that it would involve some form of try-catch statement to catch any potential exceptions. Maybe the naming is the missing piece here.

@Jhabkin
Copy link
Contributor Author

Jhabkin commented Jan 9, 2024

For example you try to get data from repository, don't get it and try to fetch data from remote service:
userRepository.find(userId).tryRecoverIf({ it is NotFoundError }) { userMasterSystem.getById(userId) }
I didn't want to overload fun recoverIf, that's why I have prefixed it with try. I agree that it is not very clear naming. Maybe something like runOnErrorIf will be better?

@michaelbull
Copy link
Owner

michaelbull commented Jan 9, 2024

I think the names could actually be andThenRecoverIf. I've detailed how I arrived at the names below - plugging these into your example ends up with it making sense in my head and doesn't mislead about a try-catch.

Let me know if you disagree or come to a better name.


If we look at the symmetry between map and recoverIf

public inline infix fun <V, E, U> Result<V, E>.map(transform: (V) -> U): Result<U, E> {
    contract {
        callsInPlace(transform, InvocationKind.AT_MOST_ONCE)
    }

    return when (this) {
        is Ok -> Ok(transform(value))
        is Err -> this
    }
}

public inline fun <V, E> Result<V, E>.recoverIf(predicate: (E) -> Boolean, transform: (E) -> V): Result<V, E> {
    contract {
        callsInPlace(predicate, InvocationKind.AT_MOST_ONCE)
        callsInPlace(transform, InvocationKind.AT_MOST_ONCE)
    }

    return when (this) {
        is Ok -> this
        is Err -> if (predicate(error)) {
            Ok(transform(error))
        } else {
            this
        }
    }
}

Then we look at the flatMap (aka andThen):

public inline infix fun <V, E, U> Result<V, E>.andThen(transform: (V) -> Result<U, E>): Result<U, E> {
    contract {
        callsInPlace(transform, InvocationKind.AT_MOST_ONCE)
    }

    return when (this) {
        is Ok -> transform(value)
        is Err -> this
    }
}

If we apply the symmetry from the first snippet to the andThen, we would get:

public inline infix fun <V, E, U> Result<V, E>.andThenRecoverIf(predicate: (E) -> Boolean, transform: (E) -> Result<U, E>): Result<U, E> {
    contract {
        callsInPlace(transform, InvocationKind.AT_MOST_ONCE)
    }

    return when (this) {
        is Ok -> this
        is Err -> if (predicate(error)) {
            transform(error)
        } else {
            this
        }
    }
}

Which is identical to your tryRecoverIf.

This would result in your example producing the following code:

userRepository
    .find(userId)
    .andThenRecoverIf({ it is NotFoundError }) { userMasterSystem.getById(userId) }

@Jhabkin
Copy link
Contributor Author

Jhabkin commented Jan 9, 2024

andThenRecoverIf sounds nice to me. Should I move it to And.kt?

@michaelbull
Copy link
Owner

Yeah I think that makes sense. I feel like the lib is quickly exploding with the if/unless variations for every function so it's likely these will be the last of their type.

@michaelbull
Copy link
Owner

Seems like one of the tests has failed on macos-11

@Jhabkin
Copy link
Contributor Author

Jhabkin commented Jan 29, 2024

Yes, I missed it, thank you! Hope no errors now =)

@michaelbull
Copy link
Owner

Merged in d4414b1

@michaelbull michaelbull closed this Feb 6, 2024
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

Successfully merging this pull request may close these issues.

2 participants