[darcs-users] darcs patch: Use unless. (and 2 more)
Trent W. Buck
trentbuck at gmail.com
Thu Jan 15 10:23:13 UTC 2009
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.
More information about the darcs-users
mailing list