Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat(filter): allow filtering of an object #5451

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions src/ng/filter/filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
* @kind function
*
* @description
* Selects a subset of items from `array` and returns it as a new array.
* Selects a subset of items from `collection` and returns it as a new array.
*
* @param {Array} array The source array.
* @param {Array|Object} collection The source collection.
* @param {string|Object|function()} expression The predicate to be used for selecting items from
* `array`.
* `collection`.
*
* Can be one of:
*
Expand All @@ -19,7 +19,7 @@
* will be returned. The predicate can be negated by prefixing the string with `!`.
Copy link
Member

Choose a reason for hiding this comment

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

All occurrences of array should be replaced with collection.
It is also not clear if you are matching against the keys or the values in case collection is an object.

*
* - `Object`: A pattern object can be used to filter specific properties on objects contained
* by `array`. For example `{name:"M", phone:"1"}` predicate will return an array of items
* by `collection`. For example `{name:"M", phone:"1"}` predicate will return an array of items
* which have property `name` containing "M" and property `phone` containing "1". A special
* property name `$` can be used (as in `{$:"text"}`) to accept a match against any
* property of the object. That's equivalent to the simple substring match with a `string`
Expand All @@ -28,12 +28,12 @@
* not containing "M".
Copy link
Member

Choose a reason for hiding this comment

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

not containing "M"

This seems wrong.

Copy link
Author

Choose a reason for hiding this comment

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

Can you expand? I'm not sure what's wrong about it. (This is also what's on master)

Copy link
Member

Choose a reason for hiding this comment

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

Ooops ! I was tricked by github's hiding 2 lines in between. I was reading:

That's equivalent to the simple substring match with a string
not containing "M".

So, it's fine as it is :)

*
* - `function(value, index)`: A predicate function can be used to write arbitrary filters. The
Copy link
Member

Choose a reason for hiding this comment

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

It might make sense the chage the name of the second arguments from index to key (or something better).

* function is called for each element of `array`. The final result is an array of those
* elements that the predicate returned true for.
* function is called for each element of `collection`. The final result is an array of those
* values that the predicate returned true for.
*
* @param {function(actual, expected)|true|undefined} comparator Comparator which is used in
* determining if the expected value (from the filter expression) and actual value (from
* the object in the array) should be considered a match.
* the object in the collection) should be considered a match.
*
* Can be one of:
*
Expand Down Expand Up @@ -116,8 +116,8 @@
</example>
*/
function filterFilter() {
return function(array, expression, comparator) {
if (!isArray(array)) return array;
return function(collection, expression, comparator) {
if (!isObject(collection)) return collection;

var comparatorType = typeof(comparator),
predicates = [];
Expand Down Expand Up @@ -208,15 +208,14 @@ function filterFilter() {
predicates.push(expression);
break;
default:
return array;
return collection;
}
var filtered = [];
for ( var j = 0; j < array.length; j++) {
var value = array[j];
if (predicates.check(value, j)) {
forEach(collection, function(value, index) {
if (predicates.check(value, index)) {
filtered.push(value);
}
}
}, this);
return filtered;
};
}
6 changes: 6 additions & 0 deletions test/ng/filter/filterSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ describe('Filter: filter', function() {
expect(filter(items, {name: 'b'})[0].name).toBe('abc');
});

it('should filter an object', function() {
var items = {0: {name: 'a'}, 1: {name: 'abc', done: true}};
expect(filter(items, function(i) {return i.done;}).length).toBe(1);
expect(filter(items, 'a').length).toBe(2);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should add a proper test for what the data should look like --- just making assertions about the length isn't really that useful

});

it('should take function as predicate', function() {
var items = [{name: 'a'}, {name: 'abc', done: true}];
expect(filter(items, function(i) {return i.done;}).length).toBe(1);
Expand Down