1
\$\begingroup\$

I use a lot of lambda expressions in C#, so I've decided to create some similar functionality in JavaScript. Since jQuery is included in my projects, I'm using $.grep() instead of Array.prototype.filter() to support older browsers without including shims.

The two examples below corresponds to Where() and FirstOrDefault() in C#:

if(!Array.prototype.where){

    Array.prototype.where = function (func) {

        var that = this;

        switch (typeof func) { 

            case 'function': 
                return $.grep(that, func); 

            case 'object':

                for(var prop in func){
                    that = $.grep(that, function(item){
                        return item[prop] === func[prop];
                    });
                }

                return that;

            default: 
                throw new ReferenceError("no/wrong func type passed"); 
        }
    };
}

if(!Array.prototype.firstOrDefault){

    Array.prototype.firstOrDefault = function(func){
        return this.where(func)[0] || null;
    };
}
var persons = [{ name: 'foo', age: 1 }, { name: 'bar', age: 2 }];

var where1 = persons.where(function(p){
    return p.age === 1 && p.name === 'foo';
});

console.log(where1);

var where2 = persons.where({ age: 1, name: 'foo' });

console.log(where2);

var fod1 = persons.firstOrDefault(function(p){
    return p.age === 1 && p.name === 'foo';
});

console.log(fod1);

var fod2 = persons.firstOrDefault({ age: 1, name: 'foo' });

console.log(fod2);

Fiddle

I would have preferred to add the methods as non-enumerable using Object.defineProperty, but since I need to support IE >= 7, that's not an option.

However, the only downside with having the methods enumerable would be if someone is iterating an array using for..in, which they shouldn't do in the fist place.

In case of an object being passed as argument, one way to decrease the total iterations from array length * object property count to array length * 1 could be solved using the function constructor:

var o = { foo: 1, bar: true, s: 's' };

var str = Object.keys(o).reduce(function (prev, key) {

    if(typeof o[key] === 'string')
        o[key] = '\'' + o[key] + '\'';
    return (prev ? prev + ' && ' : '') + 'x.' + key + ' === ' + o[key];

}, '');

var f = new Function('x', 'return ' + str + ';');

// pass f to filter/$.grep
console.log(f);

Fiddle

I would like some feedback about possible performance improvements and what you think about the code in general (but not pros and cons with extending native objects).

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Looks OK to me. I don't know that you could do anything to really squeeze more performance out of this. In the end you're mostly dependent on how $.grep performs.

I set up a jsperf test for your own alternatives and the code below, and constructing a function is by far the fastest (I put the function construction into where, and avoided using reduce since you said you'd need IE7+ support).

But! Constructing a function from text is just tremendously fragile:

  • Quoting a string is a pain. If the string contains single or double quotes, you can't properly quote it without a lot of parsing and escaping. You just really shouldn't try.

  • Your 'x.' + prop accessing can easily fail too, if the key isn't a proper "word". Any string can be a property name, including strings with spaces, reserved words, operators, etc.. You end up with code like x.hey look a string key === ...., which won't get you very far.
    Using subscripting access, i.e. 'x["' + prop + '"]', will get you a bit further, but you're just back to the issue of quoting strings again.

  • Only primitive values can be easily compared; you'd still need a custom function for everything else.

  • You're changing the object that's been passed in; specifically, string values get quoted. I assume that's partly just because it's an example, but still. (I avoided this in the jsperf code.)

So yeah, it's much faster to use a function, but I'd still very much avoid constructing one on the fly. Better to just write one when necessary, or accept the performance hit otherwise.

As for the existing code, I like the iterative way of reducing the matches if the argument is an object. But I wanted to try turning the loops inside out, as I find that more straightforward:

return $.grep(this, function (item) {
  for(var prop in func) {
    if(item[prop] !== func[prop]) {
      return false;
    }
  }
  return true;
});

However, doing so is slower (somewhat to my surprise).

Otherwise, the code's fine, I only have a couple of notes.

  • The that variable would usually imply that you're dealing with scopes, but that isn't the case here; You can use plain this anywhere in your function. You do need a separate variable for the object-based filtering, but I'd just do something like var filtered = this.

  • You should throw a TypeError instead of a ReferenceError. And I'd write a slightly nicer and more helpful error message, such as "func must be either a function or an object of properties and values to filter by".

  • You'll probably want to add a func.hasOwnProperty(prop) check to avoid accidentally looping through a bunch of inherited properties. It's not an issue for object literals, but it can be for other objects.

  • You'll probably want to ensure that a copy of the original array in returned, even if nothing's been filtered. This isn't a problem if you pass in a function, but could happen if you pass in an empty object.

  • It's a tad heavy on the whitespace, if you ask me, but to each their own.

In the end, I get this

Array.prototype.where = function (filter) {
    switch(typeof filter) { 
        case 'function': 
            return $.grep(that, filter); 

        case 'object':
            var filtered = this;
            for(var prop in filter) {
              if(!filter.hasOwnProperty(prop)) {
                  continue; // ignore inherited properties
              }
              filtered = $.grep(filtered, function (item) {
                  return item[prop] === filter[prop];
              });
            }
            return filtered.slice(0); // copy the array

        default: 
            throw new TypeError("func must be either a function or an object of properties and values to filter by"); 
    }
};
\$\endgroup\$
2
  • \$\begingroup\$ Yeah, I guess it's better to take a performance hit than dealing with all the issues related to constructing the function from a string. Could you please elaborate the part about returning a new copy of the array? Could you show me a fiddle where it could become an issue not to copy it? Both $.grep and filter would return a new array AFAIK. Thanks for your input, I really appreciate it! \$\endgroup\$
    – Johan
    Commented Oct 27, 2014 at 13:20
  • \$\begingroup\$ @Johan Here you go. The point is just that your function always returns a new array except if you pass an empty object as your filter (because in that case, you never call $.grep). In that single edge case, you just get the original array reference back, and any modifications you make will affect the original array. Could be a really nasty bug to track down later. \$\endgroup\$
    – Flambino
    Commented Oct 27, 2014 at 13:30

Your Answer

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

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