javascript function opinions

Question

I've written two Array methods that I think will be pretty useful for the work I'm doing. Modeled after Ruby's array methods, here are my find and find_all methods. Just thought I'd toss them to the community and get some feedback. I'm pretty new to JS programming so I'm probably not using enough defensive mechanisms and maybe there are some optimizations to be made. Any thoughts?

Array.prototype.find = function(attrs){

    // If an object wasn't passed in return false, maybe throw format exception? 
    // or do primitive type search, but that's not really useful as you're passing in the very value you're looking for
    if (typeof attrs != 'object'){
     return false;
    }else{
     for(var i=0; i < this.length; i++){
      if (typeof this[i] != 'object'){
       return false;
      }else{
       var match = true;
       // Loop through all attributes of the parameters object and test for existence and match
       for(item in attrs){
        match = match && (this[i][item] && (this[i][item] === attrs[item]) );
       }
       if(match){
        return this[i];
       }
      }
     }
     // default return if no items found
     return false;
    } 
};

// find_all behaves similarly to find only returns all matched objects
// See ruby's find_all method on arrays
Array.prototype.find_all = function(attrs){
    // If an object wasn't passed in return false, maybe throw format exception? 
    // or do primitive type search, but that's not really useful as you're passing in the very value you're looking for
    if (typeof attrs != 'object'){
     return false;
    }else{
     var valid_items = [];
     for(var i=0; i < this.length; i++){
      if (typeof this[i] != 'object'){
       return false;
      }else{
       var match = true;
       // Loop through all attributes of the parameters object and test for existence and match
       for(item in attrs){
        match = match && (this[i][item] && (this[i][item] === attrs[item]) );
       }
       if(match){
        valid_items.push(this[i]);
       }
      }
     }
     return valid_items;
    } 
}

Some examples:

var a={id:1,parent_id:2}
var b={id:2,parent_id:3}
var c={id:3,parent_id:2}
var arr = [a,b,c]

arr.find({parent_id:2}) 
// Object id: 1 parent_id: 2

arr.find_all({parent_id:2})
// [Object id: 1 parent_id: 2, Object id: 3 parent_id: 2 ]

Answer 1

Instead of "typeof attrs != 'object'", it is much more common to see:

if (attrs == null){
    return false;
}

(You could also use "if (attrs == undefined)").

Answer 2

These might be useful functions for you, but I would reconsider adding them to the Array prototype -- they just don't seem general-purpose enough for that since they only look for object property values. I would just use them as utility functions (passing in the array) or add them to some utility class. When I think of an array find method, I think of something more like the example here: http://www.hunlock.com/blogs/Mastering_Javascript_Arrays (but then I'm not familiar with Ruby's version).

Answer 3

Your function modifies the prototype Array. Some libraries feel this is okay (Prototype). Others are opposed (jQuery) to this global modification.

See this page at the Prototype website. You'll see why they feel that it's ok to modify the Array prototype but only if you're not using for .. in loops to iterate [edit: over arrays (which you're right, you're not doing)].

I actually prefer jQuery's decision to leave it alone given that you want your code to be reusable and to play nice with other code.

Also taking a look at the find algorithm, if this[i] has more properties than attrs, as long as the properties that they do have in common match, it will return true, but it would be false.

To do a complete comparison, you would have to check the length also, but objects don't have a length property, so you would have to count this[i] in one loop and attrs length separately.

Edit: Just looked at your examples. It looks like you are aware of the two objects not being entirely equal. Are you sure that you want {id:1,parent_id:2} to be found when searching for {parent_id:2}?? It seems more like that you're asking if the obj param is equal to a subset of properties of any object in an array of objects. You want to call this "find"? Is that really want you want to do? I would think find should only return if they are entirely equal or call it something else.

Edit 2: Another thought. I would definitely have find_all call find as its underlying mechanism. That way you don't repeat your code, and you'll more likely guarantee that anything find would match, find_all would match. You don't want differences to crop up between their underlying search algorithm. Although to do that you may have to refactor find slightly to return the index of the object rather than the object, which shouldn't be a problem.

Answer 4

In addition to what the other answers have said, here are some non-JS specific things about your code:

When you have an if-clause which returns, you don't need an else.

if(foo) {
    return false;
}

//other code here

This will make your code more readable as there will be less indentation/levels to keep track of inside your head.

Your functions don't follow the usual JavaScript naming convention. If you look at any JS built in methods, they use camelCase and not underscore_separated. Following the conventions of a language is always a good idea.