Hello, On Saturday 17 January 2009 15:21, Trent W.Buck wrote: > Thanks to Ferenc Wagner for the suggestion. > > Note: I'm not spoofing the author field because it seems to annoy > people when the patch is merely *derived* from the spoofee, rather > than being their work verbatim. > > Sat Jan 17 21:29:28 EST 2009 Trent W. Buck <[email protected]> > * Refactor add_to_list. > >
> New patches: > > [Refactor add_to_list. > Trent W. Buck <[email protected]>**20090117102928 > Ignore-this: 73c639dcba23a2abd3577e1d1e593384 > ] hunk ./src/Darcs/Repository/Prefs.lhs 380 > then mWriteBinFile (fp2fn $ prefs ++ p) (unlines ls) > else return () > > +-- | Add element @x@ to list @xs@ if it isn't there yet. > add_to_list :: Eq a => a -> [a] -> [a] > hunk ./src/Darcs/Repository/Prefs.lhs 382 > -add_to_list s [] = [s] > -add_to_list s (s':ss) = if s == s' then (s:ss) else s': add_to_list s ss > +add_to_list x xs = if x `elem` xs then xs else x : xs > > def_prefval :: String -> String -> IO String > def_prefval p d = do I am nervous about this change: (1) Is there a unit test for this function? (2) The new function generates lists with elements in a different order than the original, so are any users of this function depending on this order? (3) The "laziness" of the function has changed, there is perhaps some technical term for this, but as I see it, the function doesn't "let go" of the list, before it has actually verified that the element is not there, which will affect performance. I observe that this is in the area of "preferences", so off-hand, performance is perhaps not important. On the other hand, as far as I can see, the order of elements will be visible to user in some cases. Thanks and best regards Thorkil _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
