javascript function opinions

Question

J'ai écrit deux méthodes Array qui, je pense, seront très utiles pour le travail que je fais. Modelé après les méthodes de tableau de Ruby, voici mes méthodes find et find_all. Je pensais juste que je les jetterais dans la communauté et que j'aurais des réactions. Je suis assez nouveau dans la programmation JS, je n'utilise probablement pas assez de mécanismes défensifs et peut-être qu'il y a des optimisations à faire. Des pensées?

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; 
    } 
} 

Quelques exemples:

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

Au lieu de "typeof attrs = 'objet'!", Il est beaucoup plus fréquent de voir:

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

(Vous pouvez également utiliser " if (attrs == indéfini) ").

Answer 2

Ces fonctions pourraient être utiles pour vous, mais je reviendrais sur leur ajout au prototype de tableau - elles ne semblent pas assez générales pour cela car elles ne recherchent que des valeurs de propriété d'objet. Je voudrais simplement les utiliser comme des fonctions d'utilité (passant dans le tableau) ou les ajouter à une classe d'utilité. Quand je pense à une méthode de recherche de tableau, je pense à quelque chose de plus comme l'exemple ici: http://www.hunlock.com/blogs/Mastering_Javascript_Arrays (mais alors je ne suis pas familier avec la version de Ruby).

Answer 3

Votre fonction modifie le module prototype. Certaines bibliothèques pensent que c'est correct (Prototype). D'autres sont opposés (jQuery) à cette modification globale.

Voir this page sur le site Web de Prototype. Vous verrez pourquoi ils pensent que c'est correct de modifier le prototype de tableau, mais seulement si vous n'utilisez pas .. dans les boucles pour itérer [modifier: sur les tableaux (ce que vous avez raison, vous ne faites pas) ]

En fait, je préfère la décision de jQuery de le laisser tranquille, étant donné que vous voulez que votre code soit réutilisable et qu'il fonctionne bien avec d'autres codes.

En regardant aussi l'algorithme de recherche, si ce [i] a plus de propriétés que attr, tant que les propriétés qu'ils ont en commun correspondent, il retournera vrai, mais ce serait faux. Pour faire une comparaison complète, vous devez également vérifier la longueur, mais les objets n'ont pas de propriété de longueur, donc vous devez compter ce [i] dans une boucle et attr la longueur séparément.

Editer: Il suffit de regarder vos exemples. Il semble que vous soyez conscient que les deux objets ne sont pas entièrement égaux. Etes-vous sûr de vouloir trouver {id:1,parent_id:2} lorsque vous recherchez {parent_id:2}? Il semble plutôt que vous demandiez si le paramètre obj est égal à un sous-ensemble de propriétés d'un objet dans un tableau d'objets. Vous voulez appeler cela "trouver"? Est-ce que c'est vraiment ce que tu veux faire? Je pense que find ne devrait revenir que si elles sont entièrement égales ou appelez ça autrement.

Édition 2: Autre idée. Je voudrais absolument find_all appeler find comme mécanisme sous-jacent. De cette façon, vous ne répétez pas votre code, et vous aurez plus de chances de garantir que tout ce que find trouvera, find_all correspondra. Vous ne voulez pas que les différences apparaissent entre leurs algorithmes de recherche sous-jacents. Bien que pour faire cela, vous devrez peut-être refactoriser trouver légèrement pour retourner l'index de l'objet plutôt que l'objet, ce qui ne devrait pas être un problème.

Answer 4

En plus de ce que les autres réponses ont dit, voici quelques choses spécifiques non-JS sur votre code:

Lorsque vous avez un si article qui retourne, vous ne pas besoin d'un autre.

if(foo) { 
    return false; 
} 

//other code here 

Cela rendra votre code plus lisible car il y aura moins indentation/niveaux pour garder une trace de l'intérieur de votre tête.

Vos fonctions ne respectent pas la convention de dénomination JavaScript habituelle. Si vous regardez n'importe quelle méthode construite par JS, ils utilisent camelCase et non underscore_separated. Suivre les conventions d'une langue est toujours une bonne idée.