-
-
Notifications
You must be signed in to change notification settings - Fork 87
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
feature(#174): add omitIndexes #180
Conversation
562a6aa
to
3af8d6f
Compare
@BjornMelgaard What about this version ? const { contains, curry, addIndex, reject } = require('ramda');
// helpers
const rejectIndexed = addIndex(reject);
const containsIndex = curry((indexes, val, index) => contains(index, indexes));
const omitIndexes = curry((indexes, list) => rejectIndexed(containsIndex(indexes), list)); Seems more declarative, easier to read and predicate function creation is hidden inside currying and not exposed to business logic. |
@char0n yes, like it very much |
2d43ad1
to
299c257
Compare
src/omitIndexes.js
Outdated
* | ||
* RA.omitIndexes([-1, 1, 3], ['a', 'b', 'c', 'd']); //=> ['a', 'c'] | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls move rejectIndexed
and containsIndex
before the jsdoc to that jsdoc is applied directly to omitIndexes
and not to rejectIndexed
.
const expected = ['a', 'c']; | ||
|
||
eq(RA.omitIndexes(indexes, arr), expected); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls add tests for 3 more usecases:
- empty indexes (will return unmodified list)
- empty list (will return empty list)
- combination of empty list and empty indexes
Just to be sure we have corner cases covered. I will adjust pickIndexes
in #181 to be consistent with your omitIndexes
.
Then we can merge.
@char0n done |
src/index.d.ts
Outdated
* Returns a partial copy of an array omitting the indexes specified. | ||
*/ | ||
omitIndexes<T>(indexes: number[], list: Array<T>): Array<T>; | ||
omitIndexes(indexes: number): <T>(list: Array<T>) => Array<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be indexes: number[]
?
src/omitIndexes.js
Outdated
* | ||
* @func omitIndexes | ||
* @memberOf RA | ||
* @since {@link https://char0n.github.io/ramda-adjunct/1.18.1|v1.18.1} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're at 1.19.0. You can always find the next version inside package.json.
5f2b439
to
da41285
Compare
@char0n done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done
@char0n thanks) |
No description provided.