2

Would you do this sort of thing?

    var getBoard1 = function(id) {
        return $.grep(me.boards, function (board) {
            return board.Id == id;
        });
    };

Or this sort of thing?

    var getBoard2 = function(id) {
        for (var i = 0; i < me.boards.length; i++) {
            var board = me.boards[i];
            if (board.Id == id)
                return board;
        }
        return null;
    };

And why, in the context of correctness, readability and performance would you prefer that way? If you would rather do it in a third way, please share.

Gaute Løken
  • 7,522
  • 3
  • 20
  • 38

4 Answers4

3

This is what the grep function looks like (jQuery v1.8.2):

grep: function( elems, callback, inv ) {
    var retVal,
        ret = [],
        i = 0,
        length = elems.length;
    inv = !!inv;

    // Go through the array, only saving the items
    // that pass the validator function
    for ( ; i < length; i++ ) {
        retVal = !!callback( elems[ i ], i );
        if ( inv !== retVal ) {
            ret.push( elems[ i ] );
        }
    }

    return ret;
}

Essentially, you're doing the same so it wouldn't be much of a difference when it comes to performance. When I look at the jQuery code, they always return an array, where you return null. If you're using jQuery already, I would go for the jQuery version since it's better readable, otherwise go with native code.

* -- EDIT -- *

When looking at the code, this made me realize it does make a difference. Your code already returns (and finishes the loop) when it found the first item (expecting only one single result), where jQuery loops through all the items. So, if you expect only one result, your version would be faster.

MarcoK
  • 6,090
  • 2
  • 28
  • 40
2

jQuery provides convenience methods, in the background it will probably do something similar. If you are already using jQuery then you can take advantage of this, I would however not include jQuery just for one bit of code like this. It entirely depends on your situation.

As for performance, try it, see what your results are.

Jon Taylor
  • 7,865
  • 5
  • 30
  • 55
1

If you already have a dependency on jQuery then do it the first way because it's shorter and reads easier. In the very unlikely case that this function is your bottleneck and performance is not acceptable then you can start thinking about alternate implementations.

If you don't already depend on jQuery then the second version is preferable because the tradeoff (including jQuery vs writing a few more lines of code) is not worth it.

Jon
  • 428,835
  • 81
  • 738
  • 806
  • Interresting. Would you say that getBoard1 is more readable to a novice with no background with nix? I would think grep would hit google pretty fast... Short is good, but explicit can be equally good, no? – Gaute Løken Feb 01 '13 at 10:59
  • @Mithon: Depends on the novice. You don't need to know jQuery to recognize a predicate callback, and if you do it's obvious what the method does even if `$.grep` is a complete unknown. But that's not what I had in mind when answering -- the question doesn't say "what code would you prefer if a novice is going to maintain it". – Jon Feb 01 '13 at 11:02
  • no, but I did mention readability. I guess I assumed readability is doubly important when dealing with novices, so it was implied. My bad. However if it's easier for novices it will also be easier for the seniors. :) – Gaute Løken Feb 01 '13 at 11:04
  • 1
    @Mithon: Not necessarily... client-side JS is all about callbacks, and for seniors IMHO it's better for the code to be idiomatic. They should be reading simple code like this as if it were their native tongue. – Jon Feb 01 '13 at 11:06
  • You do have a good point. Won me over on the readability part. However, 5.4 times slowdown.. nopes.. native it is – Gaute Løken Feb 01 '13 at 11:09
0

I would use the native Array.filter() which is probably the fastest if you don't care for Browsersupport (IE8- will die on this).

a.filter(function(e){return e.id == id});

This returns, same like jQuerys grep an array where you would have to fetch the first value.

Christoph
  • 50,121
  • 21
  • 99
  • 128
  • Though this looks good at first glance, you'd have to add code to check if the resulting array is non-empty, and return the first element if it isn't. So the cleanliness of it would get lost to that. – Gaute Løken Feb 01 '13 at 11:19
  • well, a check would have to be done in every case, regardless of which method you take and it really doesn't matter if you write `if(board)` (custom method checking for null) or `if(board[0])` (.filter), does it? Also jQuerys grep also returns an array so you need to check this too with `if(board[0])`:-p – Christoph Feb 01 '13 at 11:24
  • Sure it does. In both senses. If the method returns an array, those if's will return different results. If you mean doesn't make a difference if we change the meaning of getBoard to return an array rather than a single board, then I strongly disagree. A function named like that definately will let people assume they'll get a single board, rather than an array, and so the next programmer that comes along a few months later will likely write the if-test wrong. And that next programmer could be myself. – Gaute Løken Feb 01 '13 at 11:28
  • @Mithon well, apparently you don't know how jQuerys grep works. In fact **it returns an array just like Array.filter does**. So the necessary check is excatly the same for both methods... if you like, wrap it in an additional function wrapper `return a[0] || null` then you have your scalar return value. Now feel free to revoke your -1. – Christoph Feb 01 '13 at 11:31
  • I do. Though I didn't realize immediately. See my comment to David above. And yes that would muddy the getBoard1 in the same way your solution would be slightly muddier. :) For the record, I didn't give you that downvote. In fact I'll give you an up. After all I did ask for alternate ways. – Gaute Løken Feb 01 '13 at 11:36
  • For compatibility, there's a native-safe js-only implementation of `Array.prototype.filter` here: https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Array/filter – Matt Sach Feb 01 '13 at 12:31
  • @Matt The problem with this is, that the speedadvantage will be at least nullified when using this shim, which is the reason I proposed this way in the first place;) – Christoph Feb 01 '13 at 12:55
  • @Christoph: Granted it would be slower than native, but the shim won't override any native implementation, so all it will do is provide (slow) compatibility for IE8 and below, leaving good browsers to use the fast version :) – Matt Sach Feb 01 '13 at 17:15
  • @Matt [I did some testing](http://stackoverflow.com/q/14647470/1047823) and funnily the native method is really slow... – Christoph Feb 01 '13 at 17:50