more efficient genBoundedEnum #33
Conversation
Should reduce time and space complexity to that of chooseInt for most BoundedEnum instances.
| bottomInt = fromEnum (bottom :: a) | ||
| enumRange = topInt - bottomInt | ||
| in if enumRange == unwrap (cardinality :: Cardinality a) | ||
| then unsafePartial $ fromJust <<< toEnum <$> chooseInt bottomInt topInt |
There was a problem hiding this comment.
This actually is unsafe though, if the Enum instance is bad, right?
There was a problem hiding this comment.
If you excluded bottom, and added it back in fromMaybe, this could be safe though.
There was a problem hiding this comment.
But then it might be harder to ensure this is uniformly distributed :(
| let topInt = fromEnum (top :: a) | ||
| bottomInt = fromEnum (bottom :: a) | ||
| enumRange = topInt - bottomInt | ||
| in if enumRange == unwrap (cardinality :: Cardinality a) |
There was a problem hiding this comment.
Shouldn't this always be the case?
There was a problem hiding this comment.
My idea here was that if the Enum instance is bad, this test will fail. If it's good, then the test passes and the following line is safe. I'll rethink this... I'm probably missing something.
There was a problem hiding this comment.
Ah, I see what you mean... A bad Enum instance could just be lying about its Cardinality. Argh.
|
I definitely see your points, above. At the same time, I feel like this is pretty useful for the majority of well-behaved |
|
If there is a need for this for performance reasons, then I would be okay with putting it in an unsafe module. Note it really is unsafe, because nothing in the |
|
Yes, this makes sense. I do feel that because generating random things is something that can often happen in a loop, it makes some sense to make this version available somewhere, with clear documentation explaining its lack of safety. I'll take another pass at this soon. |
|
I'm closing this. I think there is a more principled way to do this. |
Should reduce time and space complexity to that of
chooseIntfor mostBoundedEnuminstances.