[darcs-devel] darcs patch: allow submitting patches via HTTP POST in addition to ...

[David Roundy]

On Tue, Apr 01, 2008 at 04:00:04PM -0700, John Meacham wrote:
> On Tue, Apr 01, 2008 at 02:54:17PM +0100, Eric Kow wrote:
> > >  I actually really dislike how we pass [DarcsFlag] all over the place, as
> > >  it's a very un-typed approach.  You can't tell what information you're
> > >  passing in, or why, or what effect it will have.
> > 
> > I wonder if the implicit parameters extension could be useful for
> > helping in such a cleanup?
> > http://www.cs.chalmers.se/~rjmh/Globals.ps
> 
> gawd no. Implicit parameters are a horrible hack, I hope ghc gets rid of
> them so people stop getting tempted to use them and code themselves into
> a corner. they break beta reduction and complicate type signatures,
> their global namespace also makes code that uses them very non-modular.

We're in agreement here.

> A reader monad is very canonical haskell here.

I really would prefer to avoid using a reader monad.  Admittedly, my only
experience with them is from xmonad, but my preference is strongly in favor
of the use of explicit parameters.

> data Env = Env {
>         envFlags :: [DarcsFlags]
>         }
> 
> newtype DIO a = DIO (ReaderT Env IO a)
>         deriving(Monad,Functor,MonadIO, Reader Env)
> 
> putInfoLn :: String -> DIO ()
> putInfoLn s = do
>         flags <- asks envFlags
>         unless (Quiet `elem` flags) $ putStrLn s 
> 
> 
> things like the http stream cache can also be piped around easily in a
> similar fashion.

I'm not sure what the http stream cache would be, but is there any reason
not to just put it into a global variable? It's a bit, but it is also
modular (the ugliness doesn't affect more than the one module in which it's
implemented) and lazy, and has worked for us pretty well so far.

> now you can just use DIO in some places. I find it makes it a _lot_
> easier to read and understand code when the incidental environment doesn't get in
> the way of the actual input/output parameters. It is hard to tell when a
> function is meant to actually do something with the env or just needs it
> for putVerbose (which is replicated way to many times)

But your solution doesn't change anything.  There's still no way to tell
when a function is mean to do something with [DarcsFlag] or just needs it
for putVerbose.  I'd rather move in the opposite direction, and pass around
a Verbosity instead of [DarcsFlag] when possible.

And when global variables really do make sense, I'm all right with
introducing more global variables (although it's obviously better to keep
the number to a minimum).

> I don't think things accidentally ending up in DIO will be a problem,
> you already require top level type signatures so people can't
> unconciously end up with stuff in DIO that wasn't intended. Plus you end
> up getting to use all the wonderful monad combinators signifigantly
> simplifying code making it easier to grok.

I'm not aware of wonderful monad combinators... but I understand that I'm
pretty primitive when it comes to Haskell programming.

> And since the type is abstract, you end up with a lot more safety and A
> lot easier to figure out code.

I don't see where the additional safety would come in, or where code would
be easier to figure out.  Can you give an example of a bug that would be
made impossible? As far as easier-to-figure-out code, I don't see how
hiding some of the parameters to a function inside a monad makes it easier
to figure out what they are.  It just means you have to look in yet another
module in order to figure out what's going on.  And you've got all the same
issues that you've got no way to tell which of those parameters are
actually used.  It's essentially a formalization of the bad practice that
we've already got.  If we're going to change anything, I'd rather fix that
bad practice.  (e.g. passing a Verbosity around could make some function
types more transparent.)
-- 
David Roundy
Department of Physics
Oregon State University