
Hi Daniel,
Some follow up on your comments:
On Thu, Dec 31, 2009 at 5:54 AM, Daniel Fischer
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