Open mexx opened 9 years ago
So we've got a few options:
RequireQualifiedAccess
to the Trial
module (because the attribute is invalid on Active Patterns)... this will impact all of the other combinatorsRequireQualifiedAccess
to the Result
unionOk | Fail
to Ok | Bad
(or something else?)Pass | Warn | Fail
to something (I'm at a loss because "pass" and "fail" are natural antonyms)I'm honestly a fan of the option 3. But, since changing Result
is something people have had feelings about in the past, I will put to anyone interested as an informal poll.
Opinions?
@vasily-kirichenko what do you think about 3? I think this is a good solution.
@forki I'm not sure I understand the problem. Our team use the following with no problem:
let inline Ok a: Choice<_, _> = Choice1Of2 a
let inline Fail a: Choice<_, _> = Choice2Of2 a
let (|Ok|Fail|) = function
| Choice1Of2 a -> Ok a
| Choice2Of2 a -> Fail a
@forki @vasily-kirichenko the ok
and fail
functions aren't at issue. We have an Active Pattern shadowing one case of the underlying DU (because we went with a specific DU rather than aliasing Choice<'a,'b>
).
Maybe aliasing is a good idea? It'd enable many existing functions and computation expression builders from various libraries.
this would mean that we don't have messages in the success case. I actually don't think we need that. /cc @mexx @theimowski
we could leave messages in success case and do sth like
let inline Ok a: Choice<_, _> = Choice1Of2 (a, [])
are the messages in success necessary? I copied them over, here's a sample use case for those: https://github.com/swlaschin/Railway-Oriented-Programming-Example/blob/master/src/FsRopExample/DataAccessLayer.fs#L204-L205
in that cases we would just change the Success result type to a tuple, right?
I think it's a tuple already: https://github.com/fsprojects/Chessie/blob/master/src/Chessie/ErrorHandling.fs#L9
no I mean we would make 'TSuccess a tuple (which then contains the message)
Ahh yes
so I propose we change that to what @vasily-kirichenko is using.
If we drop the extra messages from Ok
, what does that mean for Pass|Warn|Fail
?
To drop or not to drop, that's a question.
For me messages of success track can be seen as a process log. Maybe we need simply a data type for this concept?
type WithLog<'Value, 'Message> = 'Value * 'Messages list
Currently the error messages and process log messages must be of same type, but is it always the case? Furthermore most of the time I don't need the process log at all, but it's there and interferes every time I only want the result, besides of using returnOrFail
.
With this concept we could go the aliasing way and let the user decide whether and when to add process log to the code.
If we drop the extra messages from
Ok
, what does that mean forPass|Warn|Fail
?
Well with the extra concept for process log those recognizers would work with Result<WithLog<'Success, 'Message>, 'Message>
type or an alias ResultWithLog<'Success, 'Message> = Result<WithLog<'Success, 'Message>, 'Message>
.
So, I played with a bit with @mexx's idea, and incorporated @vasily-kirichenko's suggestion of just aliasing Choice<'TSuccess,'TFailure>
. I have to say, I think I've missed something.
Removing the "process log" and the list of failure messages basically leaves us with a nearly straight implementation of what already exists in ExtCore (and probably FSharpX, though I haven't checked). If that's what folks want (or do they really just want Haskell?), then I'm not sure there's much merit in Chessie.
I think we're still in the early phase. What about another experiment: what if we created a third union case for the warn stuff?
What about me, I'm not going to use Chessie since I'm totally happy with the Either monad. So, don't get my opinion seriously.
With third union case all users would be forced to care about it, even when they don't want process log at all.
Not sure if it's relevant but we use
| Success<'TResult, 'DomainMessage> | Failure<'DomainMessage list>
and are watching Chessie to see if we should switch to it instead of maintaining our own (based on Scott Wlaschins demo implementation with some additional stuff added).
We use the message with the Success (Ok in Chessie's case I think) regularly and having a 3rd Union case would probably be somewhat overkill to take care of.
Just my two cents.
I've been noodling this a bit. What about an approach that mostly matches Scott Wlaschin's with some niceties overlaid? Specifically, a core union type:
type Outcome<'success,'message> =
| Pass of success:'success * warning:'message list
| Fail of failure:'message list
plus many of the functions Scott talks about here. For example:
let either withPass withFail outcome =
match outcome with
| Pass (v,log) -> withPass (v,log)
| Fail failure -> withFail failure
Then, we layer in an Active Pattern for the simpler (no log) scenario:
let (|Ok|Bad|) outcome =
match outcome with
| Pass (v,_) -> Ok v
| Fail errors -> Bad errors
And also provide some functions which accommodate the absence of the warning log. These functions would likely follow some sort of naming convention. For example:
let eitherQuiet withPass withFail outcome =
let inline withOk (v,_) = withPass v
either withOk withFail outcome
(Note: the suffix Quiet is used here for illustrative purposes only. We can surely come up with a better name.) Alternately, we can just ship two modules (one for each set of signatures). At any rate, I've got about 70% of this approach done (would send a PR at 100%). And I thought I'd see if there was any interest in continuing in this direction.
Feedback wanted.
sounds good.
Please see #22
The
Fail
active recognizer introduced in #15 by @pblasucci shadows theFail
union case. It actually introduces "incomplete pattern matches on this expression" compiler warnings for code likePrior to this change the match was complete. Fortunately the change only confuses the compiler and do not change the behavior of the pattern match.
We should require qualified access on one of the two constructs or rename one of them to avoid the name clash.