On Sun, May 17, 2009 at 21:59:59 +1000, Trent W.Buck wrote: > Yay for pointless time-wasting! The best part is, doing this exact > same refactoring before... > > Sun May 17 21:53:25 EST 2009 Trent W. Buck <[email protected]> > * Refactor authors_cmd (darcs show authors). > Adds comments and avoids icky calling concatMap (uncurry replicate) on > [(count, canonicalized name)]. Improves speed of "darcs show authors" > by a whopping seven percent (fifteen milliseconds) and probably makes > the code a lot harder to understand (not to mention longer).
Applied, thanks! The main thing this patch does is to replace replicating a canonical name n times, grouping them and then counting the occurrences with simply summing n. Refactor authors_cmd (darcs show authors). ------------------------------------------ > - else reverse $ map shownames $ sort $ > - map (\s -> (length s,head s)) $ group $ sort $ concat $ map > (\(n,a) -> replicate n a) $ > - map (\s -> (length s,canonize_author spellings $ head s)) $ group > $ sort authors > - where > - process = pi_author . info > - shownames (n, a) = show n ++ "\t" ++ a > + then authors > + else -- A list of the form ["<count> <canonical name>"]... > + > + -- Turn the final result into a list of strings. > + map (\ (count, name) -> show count ++ "\t" ++ name) $ I'm not too fond of seeing these kinds of changes unless they make a clear improvement to the code, my apologies if this is pot-kettle-black. It's fine and very good to tidy up. The objection is making minor changes in cases where the choice can easily go both ways. In other words, avoid generating churn. (that said, shownames was the wrong name for that helper function anyway, so it's probably good that you got rid of it) No comments below. > + -- Sort by descending patch count. > + reverse $ sortBy (comparing fst) $ > + -- Combine duplicates from a list [(count, canonized name)] > + -- with duplicates canonized names (see next comment). > + map ((sum *** head) . unzip) $ > + groupBy ((==) `on` snd) $ > + sortBy (comparing snd) $ > + -- Because it would take a long time to canonize "foo" into > + -- "foo <[email protected]>" once per patch, the code below > + -- generates a list [(count, canonized name)]. > + map (length &&& (canonize_author spellings . head)) $ > + group $ sort authors -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> PGP Key ID: 08AC04F9
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
