On Thu, Jan 15, 2009 at 09:30:01AM +0000, Eric Kow wrote: > Hmm... I have some minor comments. Perhaps resubmit? > I await your instructions. > > Use unless. > ----------- > > ensureDirectories :: WriteableDirectory m => FileName -> m () > > ensureDirectories d = do > > isPar <- mDoesDirectoryExist d > > - if isPar > > - then return () > > - else ensureDirectories (super_name d) >> (mCreateDirectory d) > > + unless isPar $ > > + do ensureDirectories $ super_name d > > + mCreateDirectory d > > I might be a little less enthusiastic care-free about making purely > parenthesis and do-notation vs explicit type refactors (because we can > flip-flop a bit about which is best for what situation, so it may be > good just to avoid the churn).
Here I'm basically following the Lisp style of: IF has two branches; use WHEN or UNLESS if you only have one branch. Maybe people don't buy into that philosophy for Darcs? In any case, I forgot to include "import .. unless" in this patch ^_^;; > Redundant do. > ------------- > > +syncSlurpy put s = if unsyncedSlurpySize s > slurp_sync_size > > + then return <<= put s > > + else return s > > return <<= put s could also have been simplified to put s I'll resubmit. > Haddockize add_to_list. > ----------------------- > > +-- | Add a new element to a list, as long as that element is not > > +-- already in the list. > > add_to_list :: Eq a => a -> [a] -> [a] > > add_to_list s [] = [s] > > hunk ./src/Darcs/Repository/Prefs.lhs 384 > > -add_to_list s (s':ss) = if s == s' then (s:ss) else s': add_to_list s ss > > +add_to_list s (s':ss) | s == s' = s:ss > > + | otherwise = s': add_to_list s ss > > False advertising, Well, naughty of me to have dumped the second hunk in there, anyway. > and also the kind of refactor I would have just > avoided (churn without appreciable gain in clarity, i.e. one could have > argued that the previous version (sans excess parens) would have been > more readable... I don't know which is which, but the fact that it's not > cut and dry probably means it's wisest to leave it alone) Sure thing; today I've just been exploring various stylistic refactors and trying to decide which ones appreciably increase readability. I kinda thought this one did because the plain words if, then and else don't really "stand out" for me when they're all on one line. I defer to your greater experience on this matter, however, so just ignore this patch. _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
