add support for the contains() function, fixes #88 #93
Conversation
| return Return(false) | ||
| } | ||
| var h = l.head() | ||
| return h === val ? |
There was a problem hiding this comment.
I think that we should add some helper function so we can use .equals() powers of our structures. It'll also work with other 3rd party stuff that implements Setoid.
There was a problem hiding this comment.
good point! I wasn't aware of this Setoid initiative. Nice to see that. So, we would have something like...
if (a.equals && b.equals) {
return a.equals(b);
}
return a === b;
?
not sure how it'll work for immutablejs if they don't implement Setoid though :( Maybe a user would have to add equals to their prototypes? Didn't check how their collections are implemented. Also, I guess that won't work with JS arrays out of the box?
There was a problem hiding this comment.
Immutable has quite sophisticated implementation of is-> https://github.com/facebook/immutable-js/blob/master/src/is.js
but I think we don't need that much, I think that sth like this would be enough:
if (a === b || (a !== a && b !== b)) {
return true;
}
if (!a || !b) {
return false;
}
if (isFunction(a.equals) && isFunction(b.equals)) {
return a.equals(b);
}
return false;The a !== a && b !== b is about NaN.
There was a problem hiding this comment.
hmm you surely have thought about this more than I did, but the if (!a || !b) frightens me. It makes it that "" != "", because !"" is true. Maybe we should just do === undefined or what it is you have in mind there?
There was a problem hiding this comment.
Look that if we have a = "" and b = "" it'll be returned with true from first condition.
This line is an optimisation to avoid function checks.
If you don't like it it can be dropped, but it's not that evil.
There was a problem hiding this comment.
ok i understand now. let's add this, i'll just add comments. too bad we don't (afaict) don't cover arrays though
There was a problem hiding this comment.
one can always use our or immutable's is():
arr.some(a => is(a, b))
Or we can make it curried so:
arr.some(is(a))
;)
There was a problem hiding this comment.
not sure how it'll work for immutablejs if they don't implement Setoid though :( Maybe a user would have to add equals to their prototypes?
I missed this part somehow. All ImmutableJS structures implement Setoid ;)
| @@ -627,6 +649,9 @@ | |||
| return fn(a) ? self : None() | |||
| }) | |||
| }, | |||
| contains: function (val) { | |||
| return this.isSome() ? this.val === val : false; | |||
There was a problem hiding this comment.
...and use this helper to all equality checks.
|
done. I didn't squash the commits. Maybe it could make sense to export 'areEqual' to users of the library, not sure... |
ulfryk
dismissed
their stale review
Please run npm run publ and commit changed dist/ :)
|
sorry :S when this is merged i'll rebase the forEach pull request and run publ there too |
No description provided.