On Tue, Dec 16, 2008 at 03:34:28PM +0000, Eric Kow wrote:
>
> Check out this neat DrHaskell report on the darcs code (attached).
> DrHaskell has lots of style tips for our code... what do you think?
This is probably mostly comments for Neil rather than darcs-users, but
here you go anyway:
I disagree with "Use a string literal": [Char] and String are logically
different types, even if they happen to be synonyms. I do sometimes
write a [Char] as a String literal, but I feel dirty when I do it.
Whether "Eta reduce" makes sense or not varies on a case-by-case basis,
IMO. The question is mostly, "which is easiest to read and understand?".
"Use fewer brackets" I generally agree with. Likwise "Use concatMap",
"Use replicate". "Use liftM" is certainly preferable to ">>= return".
"Use (:)" depends on context IMO. I'd generally use "xs ++ [y] ++ zs"
rather than "xs ++ y:zs".
"Use const", yeah, i guess so.
"Use isPrefixOf", I think the tool should recognise constants specially.
So
take 4 (just_name pinfo) == "TAG "
=>
"TAG " `isPrefixOf` just_name pinfo
and
take 4 (just_name pinfo) == "TAG"
is complained about loudly.
If the RHS isn't a constant then the suggested replacement behaves
differently: It evaluates the entire spine of the RHS, whereas the
original only evaluates the necessary prefix.
"Use on", yeah, I guess so.
"Redundant if", yup, but the tool has added some redundant parentheses.
"mapM/mapM_", yup.
"Lambda shift", yup.
"Use a list comprehension" I'm not convinced isn't an obfuscation.
Again, the tool has added redundant parentheses.
"Use head": The first case:
argv !! 0 == "-h" || argv !! 0
=>
head (argv !! 0 == "-h" || argv)
is a bug, but otherwise agree.
Thanks
Ian
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users
