search

Automattic / Gravatar-SDK-Android

Gravatar Android library
https://gravatar.com
Mozilla Public License 2.0
36 stars 8 forks source link

Drop Result type in favor of kotlin.Result #296

Open AdamGrzybkowski opened 1 week ago

AdamGrzybkowski commented 1 week ago

Gravata.Result type doesn't add anything extra and as per this post this will reduce the mental complexity and shrink the size of our API.

wzieba commented 1 week ago

+1. When working with Gravatar SDK, it was confusing to understand this setup as I was confusing com.gravatar.services.Result with kotlin.Result

Additionally, the custom Result class prevents me from using some helpful methods like fold.

AdamGrzybkowski commented 2 days ago

As I was reading about the Result class I started doubting this is a good idea. As per this design documment:

The Result class is designed to capture generic failures of Kotlin functions for their latter processing and should be used in general-purpose API like futures, etc, that deal with invocation of Kotlin code blocks and must be able to represent both a successful and a failed result of execution. The Result class is not designed to represent domain-specific error conditions.

That's not in line with what we do with our SDK. We try to provide users with domain-specific errors that are defined by ErrorType. Those might look generic for now, but we have more error types incoming in the new OpenAPI spec.

Later on in the document:

If there is a business need to distinguish different failures and process these different failures in distinct ways on each invocation site, then the following kind of signature shall be considered:

This seems to be our use case specifically.

@wzieba thoughts on this?