[darcs-users] darcs patch: make rollback smarter about breaking up ... (and 5 more)

Ben Franksen 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
>> one.
> 
> 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,
i.e.

  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.

Cheers
Ben



More information about the darcs-users mailing list