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

Add a recipie for checking if an object is empty #80

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

adamchester
Copy link
Member

@adamchester adamchester commented Sep 23, 2022

It took me quite a while to figure out how to conditionally check if the rest(true) object was empty, so here it is:

image

@SimonCropp
Copy link
Contributor

@adamchester is there an existing test for that scenario?

@nblumhardt
Copy link
Member

@SimonCropp I think for these the tests usually cover the individual functions and operators; the recipes are just examples. Or did I misunderstand what you mean? Cheers!

@adamchester
Copy link
Member Author

Yeah I think it’s a good call to consider if there’s a test that covers this issue.

documenting it is taking it from an accident to a thing we want to keep working?

@nblumhardt
Copy link
Member

👍

Not an accident though - just regular structural equality at work :-)

Should be nice and easy to add some to expression-cases.asv if anyone's keen! I'm on the road so won't be able to loop back for a little while :-)

@adamchester-lmg
Copy link

adamchester-lmg commented Sep 28, 2022

I found this test case:

{rest(true)}                                         ⇶ {}

I wonder if that's enough?

@nblumhardt
Copy link
Member

I think so 👍 -- that's the same logic

@nblumhardt nblumhardt merged commit ae53dab into dev Sep 30, 2022
@adamchester adamchester deleted the adamchester/check-empty-object-readme branch September 30, 2022 07:33
@nblumhardt nblumhardt mentioned this pull request Oct 10, 2022
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.

4 participants