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 aResultis 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.