Skip to content
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

introduce discardUnless #374

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

MaximilianAlgehed
Copy link
Collaborator

Closes #363

The tricky thing here is the lazy behaviour of Gen and forAll. That's what makes me unsure whether or not suchThatDiscard is a good idea in practice.

@nick8325
Copy link
Owner

What's the use case for this?

It's unfortunate that this has a similar name to suchThat and suchThatMaybe but behaves rather differently - in that it only tries the generator once but they retry until they succeed.

@MaximilianAlgehed
Copy link
Collaborator Author

I should clarify. I don't feel particularly strongly about this idea (and the name does suck, perhaps discardUnless would be better?) but I was hoping it might provide a nicer interface to discard for #363 but that's just an idea.

@MaximilianAlgehed
Copy link
Collaborator Author

I'll clarify some more. The problem raised by the issue is basically that unless (p a) discard >> pure a is a pitfall. Its clear what you intend but that is not what you get. By introducing a suchThat like thing you can express this intention in a natural way that does what you want.

We can have a separate conversation about what the role of discard is and whether or not its a good idea to use it in Gen but the problem raised by the issue is that since its there people are using it.

Copy link
Collaborator

@UlfNorell UlfNorell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think discardUnless is a better name

@MaximilianAlgehed MaximilianAlgehed changed the title introduce suchThatDiscard introduce ~suchThatDiscard~discardUnless Apr 4, 2024
@MaximilianAlgehed MaximilianAlgehed changed the title introduce ~suchThatDiscard~discardUnless introduce discardUnless Apr 4, 2024
@nick8325
Copy link
Owner

I must admit, I'm a bit sceptical that people will actually use this, just because I always find the suchThat API a bit clunky to use. But I've no objection to merging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

discard is too lazy
3 participants