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