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

Update the equals function to fall back to structural equality #154

Merged
merged 2 commits into from
Jun 19, 2021

Conversation

Avaq
Copy link
Member

@Avaq Avaq commented Apr 24, 2021

This PR competes with and so closes #147

When comparing two values that are:

  1. of the same type; and
  2. do not provide a 'fantasy-land/equals'-method; but
  3. also don't have a known type with a Z-provided implementation,

then previously Z.equals would return false, but this false is just as
reliable as a true would be.

After this PR, instead of returning false, the Z.equals now returns
the structural equality of the two inputs.

@Avaq Avaq requested a review from davidchambers April 24, 2021 10:08
@Avaq Avaq self-assigned this Apr 24, 2021
@Avaq Avaq force-pushed the avaq/structural-equality branch from 82fb5b2 to fd43ddc Compare April 24, 2021 10:11
@Avaq
Copy link
Member Author

Avaq commented Apr 24, 2021

I'm looking into what it means, conceptually, to make this same change to Z.lte.

index.js Outdated Show resolved Hide resolved
test/index.js Show resolved Hide resolved
@davidchambers davidchambers force-pushed the avaq/structural-equality branch 2 times, most recently from 1ac69a4 to cc85990 Compare April 25, 2021 21:30
@Avaq
Copy link
Member Author

Avaq commented May 1, 2021

I prefer this approach over the direction #147 is going. How do you feel about merging this @davidchambers ?

@davidchambers davidchambers force-pushed the avaq/structural-equality branch 3 times, most recently from 43bc871 to 13c24c8 Compare May 1, 2021 17:04
@davidchambers
Copy link
Member

How do you feel about merging this @davidchambers ?

I am happy with the new behaviour. I would like to spend some time reworking the function's description.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@Avaq Avaq force-pushed the avaq/structural-equality branch from 52cbd75 to 299f5c2 Compare May 2, 2021 09:01
test/index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
Avaq added 2 commits June 19, 2021 16:55
When comparing two values that are:

1. of the same type; and
2. do not provide a 'fantasy-land/equals'-method; but
3. also don't have a known type with a Z-provided implementation,

then previously Z.equals would return false, but this false is just as
reliable as a true would be.

After this commit, instead of returning false, the Z.equals now returns
the structural equality of the two inputs.
In particular, it should always be true for arbitrarily nested
structures, where some layers have Setoid and some do not.
@Avaq Avaq force-pushed the avaq/structural-equality branch from 92f1ebc to 4892eda Compare June 19, 2021 14:56
Copy link
Member

@davidchambers davidchambers left a comment

Choose a reason for hiding this comment

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

Thank you for persevering with this!

@Avaq Avaq merged commit 9e278b6 into master Jun 19, 2021
@Avaq Avaq deleted the avaq/structural-equality branch June 19, 2021 15:36
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.

2 participants