5

I have multiple filters working fine over an ng-repeat. However, the code seems unnecessarily long to actually action the filters on a set and I'm wondering if there is a better way.

Here is an example filter - this bit I'm OK with (unless anyone has any advice) - they all follow a similar structure:

app.js

.filter('taskClient', function() {
    return function (items, clientId) {
        if (!clientId) { return items; }
        var filtered = [];
        angular.forEach(items, function(item) {
            if (item.client) {
                if (item.client.id === clientId) {
                    filtered.push(item);
                }
            }
        });
        return filtered;
    }
})

And as I said - there are several of these. Then, on my ng-repeat, I implement them as so (this is the bit that seems hard to maintain and overly long and would appreciate info on any better techniques):

tasks-index.html

<md-list-item icp-task-line ng-repeat="task in TasksCtrl.tasks | taskOwner: TasksCtrl.selectedUserFilter | taskClient: TasksCtrl.clientId | taskDepartment: TasksCtrl.departmentId | taskPriority: TasksCtrl.priority | taskWithClient: TasksCtrl.withClient | taskEndDate: TasksCtrl.endDate | filter: {progress: 0} | filter: searchText | orderBy: TasksCtrl.orderTasks" class="md-2-line"></md-list-item>

Judging by how much scroll is involved here, I imagine you can see my issue with the above code. In order to get the length of tasks in view (also separated into completed, in progress etc) I have to list all the filters out again.

Is there a better way to implement these filters?

I'm also concerned that after reading this article I'm not understanding the difference between stateful and nonstateful filters - are the above filters optimised for performance?

Improve this question
8
  • Ok can you tell us the objective of having those many filter ? What are actually trying to achieve in the display process.If a sample demo is provided that would be great. Commented Sep 17, 2016 at 6:45
  • The brief was to be able to filter tasks by all of the above options - that's why they're there. However only one is applied on load - the others all through ng-models using checkboxes or drop downs. Commented Sep 17, 2016 at 9:57
  • There's a problem with using so many filters. It has to do with all the watches filters add. The more filters you pipe, the more watches you're going to have, which will result in poorer performance. I suggest you either implement a single filter that does all the tests, or a single filter that uses numerous factory filter methods instead (hold them in an array, and in the filter use 'forEach'). There are a lot of design patterns you can use for this. The important thing is to keep the number of chained filters to a minimum. Commented Sep 19, 2016 at 6:19
  • Great thanks - any chance you could put some code in an answer? Unfortunately these filters are all necessary and need to be used individually or as a combination of any number of them. I know they add too many watchers - in trying to work out a better way. Commented Sep 19, 2016 at 7:00
  • @eitanfar "The more filters you pipe, the more watches you're going to have" That's definitely not the case with stateless filters. Commented Sep 20, 2016 at 12:58

3 Answers 3

Reset to default
4
+25

you can combine all your similar filters into one, for example (I made very approximate assumptions about structure of your task model :), I inserted filter: {progress: 0} filter inside as well):

.filter('taskFilter', function() {
  return function (items, args) {
    return items.filter(function(item) {
      return (!args.selectedUserFilter || item.selectedUserFilter === args.selectedUserFilter) &&
             (!args.clientId || item.client && item.client.id === args.clientId) &&
             (!args.departmentId || item.department && item.department.id === args.departmentId) &&
             (!args.priority || item.priority === args.priority) &&
             (!args.withClient || item.withClient === args.withClient) &&
             (!args.endDate || item.endDate === args.endDate) &&
             item.progress === 0;
    });
  }
})

now HTML may look like

<md-list-item icp-task-line ng-repeat="task in TasksCtrl.tasks | taskFilter: TasksCtrl | filter: searchText | orderBy: TasksCtrl.orderTasks" class="md-2-line"></md-list-item>
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Will this improve performance / reduce watchers? And will they work together?
1. chaining filters in HTML means that output of one filter goes to the input of the next filter, each filter iterates over received array anew, combined filter will iterate only once. 2. each filter registers a watch, combined filter will register one watch. 3. combined filter may work along with other filters, and its result is exactly the same as of chained in HTML filters. plunker with both kinds of filters - plnkr.co/edit/2XUlpSz702kqcjFXsMG1?p=preview
Apologies for the delay - I just came to review the answers and this seems to be the best answer. Only question I have is that removed my current chained filters does not seem to reduce the number of watchers - is this expected? Is there another performance benefit to your approach?
3

I have two pieces of advice for you. First, if you want to get the filtered result in a variable so you don't have to apply all the filters again to get the count, you can add as filtered_result at the end of your filters and before any track by... as described in the documentation.

My other advice to you is to not force the use of filters in the HTML. It might be better in your case to create a function that filters your TasksCtrl.tasks once and saves the results off to a TasksCtrl.filteredTasks property which your ng-repeat can iterate over with way fewer watches. If you want to keep the filtering code in Angular filters, you can inject $filter and run them manually in your function.

function filterTasks(){
    var tasks = TasksCtrl.tasks;
    tasks = $filter('taskOwner')(tasks, TasksCtrl.selectedUserFilter);
    tasks = $filter('taskClient')(tasks, TasksCtrl.clientId);
    tasks = $filter('taskDepartment')(tasks, TasksCtrl.departmentId)
    tasks = $filter('taskPriority')(tasks, TasksCtrl.priority);
    tasks = $filter('taskWithClient')(tasks, TasksCtrl.withClient);
    tasks = $filter('taskEndDate')(tasks, TasksCtrl.endDate)
    tasks = $filter('filter')(tasks, {progress: 0});
    tasks = $filter('filter')(tasks, searchText);
    tasks = $filter('orderBy')(tasks, TasksCtrl.orderTasks);
    TasksCtrl.filteredTasks = tasks;
}
Improve this answer

Comments

0

I suggest you to use javascript function filter(), which is really powerful:

.filter('taskClient', function() {
  return function(items, clientId) {
    if (!clientId) { return items; }
    return items.filter(function(item) {
      return item.client && item.client.id === clientId;
    });
  }
})
Improve this answer

5 Comments

Other than chopping a couple of lines off (appreciate the info!) - that isn't changing the way the filters are applied - it's doing exactly what my current code is doing
No, it's really different: please compare forEach and filter docs... Or, try running the two implementations on quite big arrays, and see performance difference: it will be huge... :-)
Great - I will certainly do that and as I say I appreciate the info there - good to know. See editing question though where hopefully I've made it a bit clearer - it's the actual application of these filters - in the html ng-repeat that seems over the top to me and I'm wondering if there is a better way?
OK, sorry. My answer is absolutely partial... :-( But do not underestimate forEach costs... :-)
Not at all - I hadn't explained my issue properly in the question at first! I'm currently changing my filters to use .filter instead!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.