Hi Daniel, Some follow up on your comments:
On Thu, Dec 31, 2009 at 5:54 AM, Daniel Fischer <[email protected]>wrote: > Am Donnerstag 31 Dezember 2009 10:59:54 schrieb John Van Enk: > > > Hi List, > > > > > > I recently needed a ring structure (circular list with bi-directional > > > access) and didn't see anything obvious on Hackage. I threw something > > > together fairly quickly and would like some feedback before tossing it on > > > Hackage. > > > > > > I'd really appreciate if some one would: > > > > > > 1. make sure the code looks goodish (127 lines with full docs) > > I think 'left' and 'right' aren't the optimal names. But I can't think of > something clearly better either. The same applies to 'remove'. > > Please, flip the arguments in 'insert'. While ring `insert` el or insert > ring el may seem more natural (or not) and that argument order is nicer for > foldl', consistency with the argument order of Data.List.insert, > Data.Set.insert and Data.Map.insert seems far more important to me. > Done. > > 2. make sure my tests look saneish > > Sort of. The code is so short and clear that testing it at all may hint at > paranoia. > > Jokes aside, prop_balance is a consequence of prop_list > > (toList . balance . fromList === toList . fromList . toList . fromList; > > toList . fromList === id ==> toList . fromList . toList . fromList === id). > > prop_isEmpty :: [Int] -> Bool > > prop_isEmpty [] = True == (isEmpty . fromList $ []) > > prop_isEmpty l = False == (isEmpty . fromList $ l) > > prop_isEmpty [] = isEmpty . fromList $ [] > > prop_isEmpty l = not . isEmpty . fromList $ l > > or > > prop_isEmpty l = null l == isEmpty (fromList l) > > > Agreed. I actually dislike all the tests... this is actually the first package I've used QuickCheck to test with. I'm most likely going to redo the test set shortly. I'll take this advice. Thanks again. /jve
_______________________________________________ Haskell-Cafe mailing list [email protected] http://www.haskell.org/mailman/listinfo/haskell-cafe
