[darcs-users] darcs patch: make rollback smarter about breaking up ... (and 5 more)
benjamin.franksen at bessy.de
Tue Mar 17 23:28:44 UTC 2009
Reinier Lamers wrote:
>> > > clean up Depends a wee tad
>> > > --------------------------
>> > > - Left e -> throwError (MissingPatch (info hp) e)
>> > > + Left e -> Left (MissingPatch (info hp) e)
>> > Does this make the code more obscure? I don't know how strong the
>> > association of Left with "this is an error" is within the Haskell
>> > community, but to me the change looks to make things harder to
>> > understand merely to avoid importing one extra thingy.
>> I can't speak for the Haskell community, but it's pretty strong to me,
>> using the Left e for the error condition and the Right for the successful
> Also strong for me. I'd say its more Haskell-like than exceptions, they
> are impure in a way.
They are not, at least not necessarily. Even the IO monad would not
need 'real' (impure) exceptions. Imagine IO without exceptions; every
action of type IO a that now throws an exception would instead have to
return an Either IOError a instead of plain a . Then a simple wrapper
type IOWithExceptions = ErrorT IOError IO would give you back exceptions.
Note that ErrorT is a purely functional monad transformer, no side
effects. And, btw, the throwError in the above code is /defined/ as
Left , else this change would have been a type error. Also, the line before
reads: Right p' -> return p' so if you change throwError to Left ,
then you should also change return to Right .
As it stands, the code is a strange mixture of monadic exceptions and pure
Either. I think one should decide whether to write it in monadic style,
ep = hopefullyM hp `catchError` throwError . MissingPatch (info hp)
or in applicative style i.e.
case hopefullyM hp of
Right p' -> Right p'
Left e -> Left (MissingPatch (info hp) e)
One more data point: ep is used inside the function that defines it in
monadic form, e.g.
p <- ep
This suggests defining it in this form, too. I find monadic style a bit
clearer here, because it makes it obvious that all the function ep does
is wrap a possible exception inside a MissingPatch (info hp) and rethrow.
More information about the darcs-users