On Mon, Mar 10, 2008 at 5:37 PM, Mark Stosberg <[EMAIL PROTECTED]> wrote:

> I know that as potential hacker I'm turned off when I see lines like this:
>
>  wspfr _ _ NilRL _ = return Nothing
>  wspfr jn matches (p:<:pps) skipped
>
> ( from SelectChange.lhs, line 183)
>
> In the programming culture I come from, if you name a variable "wspfr",
> "nilrl", "jn", "p" or "pps", there ought to be a comment close by
> explaining what the heck you meant.


Yes, I too agree that much of the existing darcs code is too terse with
identifiers.  I think if we, as darcs hackers, made better use of Haddock
these could be fixed up quite a bit.  David has told me that he will accept
patches containing Haddock strings and a patch to add a make target for
Haddock.  I've done this in the past, but for historical reasons the patches
were not accepted.  I think it's time to rectify this and pick it up as a
habit.

And, just to help you, NilRL is actually a data constructor for a very
important custom list type used only in the darcs source base.  It
represents the empty reverse list.  You can find more information about it
(although, very few comments) in src/Darcs/Patch/Ordered.lhs.  Basically, it
is for storing objects (in reverse order) when they form a sequence
according to their types.

For example, suppose you had these three functions, f :: a -> b, g :: b ->
c, h :: c -> d.  Then you could store them in a forward list (NilFL), or a
reverse list (NilRL) with the following constructors:
f :>: g :>: h :>: NilFL
or
h :<: g :<: f :<: NilRL

In the darcs source code typically only patches are stored in forward and
reverse lists and it's the application pre- and post-contexts that are
managed by the lists.  This allows us to exploit the type system to keep
patch manipulations safe.  This should allow us to refactor more
aggressively in the future while maintaining correctness.

As for (p:<:pps), that's a bit of a Haskell-ism.  You're probably already
familiar with things like, "foo (x:xs)".  Here "(p:<:pps)" is intended to be
exactly the same.

I don't know the meaning of wspfr and jn, but given that it appears in
SelectChange.lhs I would guess that wspfr is
"with-selected-patches-from-repository" (and if I'm right, that's a better
name IMO).

This compact, cryptic notation definitely shows roots in cultural math
> equations, and in my view makes the code much less accessible.


Yes, there is quite a bit of the darcs source that is far more cryptic that
I would like.  Sometimes (all too often?) it is in the name of efficiency.
Crack open Diff.lhs for an example of where a very realistic need for
efficiency has made maintainability a nightmare.

Jason
_______________________________________________
darcs-devel mailing list
darcs-devel@darcs.net
http://lists.osuosl.org/mailman/listinfo/darcs-devel

Reply via email to