ExtCore
ExtCore copied to clipboard
Supporting Result and/or Choice
For ExtCore v1.0 -- should we support both the Result<_,_>
and Choice<_,_>
type for error-handling, or should we drop Choice<_,_>
and use only Result<_,_>
?
Pros/cons of supporting Result<_,_>
only:
- :+1: Smaller code size, less maintenance work (long-term)
- :+1: Just one way of doing things -- maybe simpler for less-experienced F# devs
- :-1:
Result<_,_>
is a struct, so doing a blanket replacement throughout code may hurt performance in cases where aResult
is created wrapping a large struct and thereby induces expensive struct copies. - :-1: Eliminating support for
Choice<_,_>
breaks backwards-compatibility with code written against current/past versions of ExtCore. Users of the library will have to to make non-trivial changes to their code in order to upgrade to ExtCore v1.0.
Pros/cons of supporting Result<_,_>
and Choice<_,_>
:
- :+1: Provides backwards-compatibility for code already written against current/past versions of ExtCore. Users should be able to (with the exception of the breaking changes defined in #47) simply upgrade to ExtCore v1.0 when it's released.
- :+1: Users can still choose between
Result<_,_>
andChoice<_,_>
and use whatever best fits their needs. - :-1: Adds additional complexity to ExtCore -- wherever we support error-handling within functions, we now need to implement those functions twice (once for
Result
, once forChoice
). This is transitive as well -- any functions that call these functions also need to be implemented twice, etc.
cc @vasily-kirichenko @7sharp9 @wallymathieu @dsyme @tihan
I've seen some non trivial code that still uses Choice, for instance Suave.
Question is how much of your own effort you want to spend (to support backwards compatibility)?
I think that perhaps @vasily-kirichenko has the most experience using a dual Result<,>
Choice<,>
ExtCore. Though I remember that there was a lot of code (in that PR) in order to support that solution. For f#+ @gusty opted to start out with changing Choice<,>
to Result<,>
and then re-add Choice<,>
based error handling with new name to support interop with libraries that still use Choice<,>
.
Perhaps a compatibility method Result.OfChoice
and a smaller set of combined builders could be used to limit the need for support?
@wallymathieu yes, I just pulled your PR, built a nuget package and have used it (Choice and Result are both there) on production with zero problems. So, I'm for just merge that PR.