2
\$\begingroup\$

I wrote a Angular filter to use filters based on properties that handles some specific properties differently.

Is there a way to improve or use some built-in Angular filter to avoid extra code?

Usage:

<div ng-repeat="item in items | myObjectFilter: { someProp: user.uid, pending: true }">

Code:

angular.module('app', []).filter('myObjectFilter', function () {
    function _filter(source, search) {
        if (!source) { return source; }

        var result = [], defaultFilters;

        //default filter setting
        defaultFilters = {
            pending: false,
            refusedAt: null
        };

        search = angular.extend({}, defaultFilters, search);

        angular.forEach(source, function (value, key) {
            var isFiltered = true;

            for (var x in search) {
                if (value.hasOwnProperty(x)) {
                    var isEqual = false;

                    // handle specific properties or simply compare
                    switch (x) {
                        case 'someProp':
                        case 'someProp2':
                            isEqual = value[x].uid == search[x];
                            break;
                        default:
                            isEqual = value[x] == search[x];
                            break;
                    };

                    isFiltered = isFiltered && isEqual;

                    if (!isEqual) {
                        break; //exit loop if filter not passed
                    } 
                }
            }

            if (isFiltered) {
                result.push(value); // add item to filterd result
            }
        });

        return result;
    };

    return _filter;
});
\$\endgroup\$
2
\$\begingroup\$

Named functions

I like the idea of named functions. It helps when you debug your app - you are getting error with name of function instead of "anonymous".

Defalt options

The default options location is good place to keep it. Filter has to be independant from controller and should manage its own data. Although, the default options object is created at every reference to filter. This is waste of resources.

angular.module('app', []).filter('myObjectFilter', function myObjectFilter() {
    //default filter setting
    var defaultFilters = {
        pending: false,
        refusedAt: null
    };

    function _filter(source, search) {
        if (!source) { return source; }
        var result = [];
        search = angular.extend({}, defaultFilters, search);
        ...

Last if

This code:

isFiltered = isFiltered && isEqual;
if(!isEqual) {
    break; //exit loop if filter not passed
}

checks for isEqual two times.

//isFiltered = isFiltered && isEqual; //you can delete this line
if(!isEqual) {
    isFiltered = false;
    break; //exit loop if filter not passed
}

My version checks it only once. I know that you don't get much performance here, but imagine situation when you would like to change the statement for making isFiltered false. You need to change condition in two places. This can lead to serious headache and long debugging in case one of them is forgotten ;)

\$\endgroup\$
1
\$\begingroup\$

Not sure why you are using named function here. I would think typical Angular syntax (except for perhaps case where you are trying to define stateful filter), would look like this:

angular.module('app', []).filter('myObjectFilter', function () {
    return function(source, search) {
       ...
    }
}

Should default values perhaps be stored on controller and read by filter?


I don't understand the need to iterate source unless source itself is an array (meaning items in your ng-repeat is an array of arrays). This filtering function will be passed one array element from items at a time.


                // handle specific properties or simply compare
                switch (x) {
                    case 'someProp':
                    case 'someProp2':
                        isEqual = value[x].uid == search[x];
                        break;
                    default:
                        isEqual = value[x] == search[x];
                        break;
                };

This code seems off to me. Perhaps controller needs to keep a map of equality callbacks for the various properties that may be used with this filtering function. I don't think these should be hard-coded in the filter.

\$\endgroup\$
  • \$\begingroup\$ The whole point of a custom filter is to remove filtering or mapping logic from the controller. That's what my filter do. Maybe default values should be in controller, but in my app would require a replication of same default values across controllers, I put it there to make my work easier. I will add a option to pass defaultValues. I'm not sure how could this work without iterate source (array of myObject) The named function is just coding style, imo it's more readable, the original code was without it. Thanks for your review. \$\endgroup\$ – m.rufca Nov 9 '16 at 23:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.