-
Notifications
You must be signed in to change notification settings - Fork 186
Simplify arbitrary instances for map and set #1166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| pure $ case mIsFirst of | ||
| Just True -> ((x : ls), rs) | ||
| Just False -> (ls, (x : rs)) | ||
| Nothing -> ((x : ls), (x : rs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems strictly less clear. It also produces more "both" results than the current code, which might be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't want another type, you could do something basic with rem inline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer a more numeric approach, I think that makes more sense than the ternary data types here.
| TwoLists l r <- arbitrary | ||
| (l, r) <- sized $ \sz0 -> do | ||
| sz <- choose (0, sz0) | ||
| let universe = [0,3..3*(sz - 1)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I wrote the original code, but I no longer remember the purpose of using products of 3. Has your work here given you any hints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not immediately; I would presume because they're not incredibly densely packed that lets other manipulations happen, but I can't say for certain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it's probably to get some gaps for testing with values in neither? I can't imagine that's the best way to do it; Please feel free to do something else about that.
| {-# LANGUAGE TypeOperators #-} | ||
| {-# LANGUAGE TypeFamilies #-} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As brought up in #1165, we would like to get tests working with MHS, which does not support type families. So this is potentially setting up a future problem.
These are over-complicated for what they need to do.