michaelbull / kotlin-result

A multiplatform Result monad for modelling success or failure operations.
ISC License
1.05k stars 63 forks source link

Please Refactor to Use Kotlin Result For 1.5+ #59

Closed craigmiller160 closed 3 years ago

craigmiller160 commented 3 years ago

So with Kotlin 1.5+, the Result type is finally "fully featured". I say that because, with the ability to use it as the return type for methods, it is truly viable as a functional error handling solution. However, having used Kotlin's Result and your Result, I must say that I like your API and feature set far better. However, a lot of the signatures class with Kotlin's (ie, their Result vs your Result) clash, so to use your library I typealias everything (ie, KtResult).

I think it would be wonderful if you were to refactor this to be built on top of the default Kotlin result, with all your great features (ie, flatMap, orElse, etc) as extension functions to it. I can see some challenges, namely that Result is a final class that cannot be extended (for Ok/Err), but I think that the outcome would be worth the effort.

Let me know your thoughts on this. I may be able to find some time to help you if you are interested.

PS. Obviously, given the scale of the breaking changes, you would have to either bump the major version, provide legacy API support, or create a separate 1.5+ artifact.

michaelbull commented 3 years ago

I've tried to model this library using the mechanism that Kotlin themselves ended up implementing (an inline class) but concluded that it isn't possible to achieve the majority of what this library achieves. A substantial amount of the extended functions, some of which you've listed, simply don't work when I tried. kotlin.Result is also bounded by Exception, which I am in strong disagreement with, and adopting it would break backwards compatibility for this library.

The Result type in the stdlib was initially implemented purely to accommodate behaviour for the kotlinx-coroutines library, and has only really been alleviated as a return type after many people requested it. I am not convinced much thought has actually been put into making it a first-class citizen within the language when you compare it to implementations provided by stdlib's from other functional languages (Scala, Rust, Haskell, etc).

Kotlin developers themselves have in-fact been on record in admitting that the kotlin.Result type is "half-baked"[1] and is likely to change in future (and therefore I would not be pushing to move a library to extend it). Barely a week after they announced that, their plans changed again and they had decided to allow people to use it as a return type[2] with no mention of increasing its scope, but instead they are exploring checked exceptions in the form of contextual receivers.

[1] https://discuss.kotlinlang.org/t/state-of-kotlin-result-vs-kotlin-result/21103/4 [2] https://discuss.kotlinlang.org/t/state-of-kotlin-result-vs-kotlin-result/21103/5

However, a lot of the signatures class with Kotlin's (ie, their Result vs your Result) clash, so to use your library I typealias everything (ie, KtResult).

You can configure your IDE to explicitly prevent suggesting importing things like kotlin.Result.

Munzey commented 3 years ago

To add to Michael's comments, I'd recommend a read of the result KEEP doc. In particular the linked section mentions that the usage pattern for this library (railway oriented programming) is not something they want to pursue as part of the language design. It doesnt seem to make sense to base this 3rd party library off their class that differs fundamentally

tom-pratt commented 3 years ago

Could you point to some instructions for the IDE setting you mentioned to help with the naming conflict. I tried to google but not quite getting it!

michaelbull commented 3 years ago

https://www.jetbrains.com/help/idea/auto-completing-code.html#exclude