2009-07-24 5 views
1

Je suis nouveau aux fermetures et ai une compréhension de «m'obtient en faisant la plupart des choses» de javascript et ainsi je me demande comment je peux améliorer etc. sur le suivant qui était moi essayant d'avoir un objet qui a un compteur ... essayant d'améliorer/approfondir ma compréhension.Javascript setTimout dans un objet/fonction aide

éditer: le code suivant fonctionne, bien sûr ... mais probablement il est faux (est-ce?) ... Je n'ai même pas près d'une idée de savoir si c'est un code correct ou incorrect .. Où puis-je améliorer ... existe-t-il un meilleur moyen d'avoir une minuterie dans un objet/une fonction?

function myObj() { 
    this.outputId = 'div_output1'; 
    this.counter = 0; 
    this.limit = 10; 
    this.count = function() { 
     // reference to self 
     var me = this; 

     if (me.doStuff(me)) setTimeout(function() { 
      me.count(); 
     },1000); 
    }; 
    this.doStuff = function(me) { 
     if (me.counter >= me.limit) { 
      document.getElementById(me.outputId).innerText = 'count reached limit'; 
      return false; 

     } else { 
      document.getElementById(me.outputId).innerText = 'count = ' + me.counter; 
      me.counter += 1; 
      return true; 
     } 
    } 
} 

// exemple d'utilisation de l'objet ...

window.onload = function() { 

    var x = new myObj; 
    x.outputId = 'div_output2'; 
    x.count(); 

    var y = new myObj; 
    y.limit = 8; 
    y.count(); 
} 
+0

Qu'est-ce qui ne va pas exactement avec le code affiché? – Triptych

+0

Je ne pense pas que quelque chose va mal peut-être ... mais entendre parler de fuites de mémoire et la complexité générale de saisir le concept ... Je suis convaincu qu'il ya probablement une meilleure façon, d'une manière plus correcte ... – davidsleeps

+0

Can vous ajoutez ce que votre problème est à la question? – Alex

Répondre

2

Vous utilisez correctement la fermeture. Parce que quand setTimeout appelle votre fonction, « ceci » sera l'objet « Fenêtre » et vous devez créer une fermeture (que vous avez fait en affectant « ceci » pour moi) et d'y accéder.

Quoi qu'il en soit, je écrirais encore votre code un peu différemment. Je ferais en sorte que doStuff s'appelle lui-même au lieu de le renvoyer vrai/faux et de décider ensuite d'appeler à nouveau doStuff ou non.

Je n'aime pas comment vous passez l'objet 'this' à cette fonction.doStuff. Il n'y a pas besoin de ça. Pour comprendre comment cela fonctionne en JavaScript, vérifiez my detailed answer on the subject.

function Counter(opts) 
{ 
    this.outputId = opts.outputId || 'div_output1'; 
    this._currentCount = 0; 
    this.limit = opts.limit || 10; 

    this.count = function() 
       {       
        this.deferDoStuff(); 
       }; 

    this.deferDoStuff = function() 
       { 
        var me = this; 
        setTimeout(function() { me.doStuff(); }, 1000); 
       }; 

    this.doStuff = function() 
        { 
         if(this._currentCount > this.limit) 
         { 
          document.getElementById(this.outputId).innerHTML = 'Count limit reached'; 
          return; 
         } 

         document.getElementById(this.outputId).innerHTML = 'count = ' + this._currentCount; 
         this._currentCount++; 
         this.deferDoStuff(); 
        }; 
} 

Utilisation:

 var x = new Counter({ 'outputId' : 'div1' }); 
     var y = new Counter({ 'outputId' : 'div2' }); 

     x.count(); 
     y.count(); 
2

J'utiliser la fermeture sur this juste pour l'appel de fonction dans la fonction de rappel. Les autres méthodes de l'objet peuvent utiliser this comme d'habitude. Le rappel nécessite une référence explicite à l'objet car il doit appeler la méthode des objets "de l'extérieur". Mais pour la méthode appelée, il s'agit simplement d'un appel de fonction normal et il peut utiliser l'this implicite pour accéder à son objet, comme d'habitude.

Je également déplacer normalement la déclaration de méthode hors du constructeur dans le prototype des objets, car il est plus clair et plus efficace:

function myObj() { 
    this.outputId = 'div_output1'; 
    this.counter = 0; 
    this.limit = 10; 
} 

myObj.prototype.count = function() { 
    // reference to self 
    var me = this; 
    var callback = function() { 
     me.count(); 
    }; 

    if (this.doStuff()) 
     setTimeout(callback,1000); 
} 

myObj.prototype.doStuff = function() { 
    if (this.counter >= this.limit) { 
      document.getElementById(this.outputId).innerText = 'count reached limit'; 
      return false; 

    } else { 
      document.getElementById(this.outputId).innerText = 'count = ' + this.counter; 
      this.counter += 1; 
      return true; 
    } 
} 
0

Certaines des façons je nettoyer le code sont:

  1. Utiliser l'encapsulation, c'est-à-dire ne pas rendre toutes vos variables & publiques. Vous avez la méthode doStuff en tant que public sur cet objet, ce qui signifie que n'importe qui peut appeler cela non seulement la méthode count. Cela s'applique également à count, limit et outputId. Au lieu de cela, ils devraient être transmis au constructeur. Faites de l'objet de référence "moi" un membre privé de la classe afin de ne pas le définir dans la méthode. Vous pouvez potentiellement avoir un problème de la façon dont vous le faites maintenant si vous appelez la méthode en utilisant call ou apply car le contexte aura changé.

par exemple.

var obj = new myObj(); 
var otherobj = {}; 
obj.call.call(otherobj); 
  1. Je voudrais aussi changer cette ligne si vous avez en nombre. Votre juste demander des ennuis !!! If (doStuff()) setTimeout (function() { me.count() }, 1000);

Voici le code avec toutes mes suggestions

function myObj(outputId, limit) { 
    var counter = 0, 
     me = this; 

    function doStuff() { 
     if (counter >= limit) { 
       document.getElementById(outputId).innerText = 'count reached limit'; 
       return false; 

     } else { 
       document.getElementById(outputId).innerText = 'count = ' + counter; 
       counter += 1; 
       return true; 
     } 
    } 
    this.count = function() { 
     if (doStuff()) { 
      setTimeout(function() { 
       me.count(); 
      },1000); 
     } 
    }; 
} 
+0

Je suppose que vous souhaitez écrire obj.count.call. De plus, je ne suis pas d'accord avec le deuxième point. Vous ne voudrez peut-être pas toujours fixer la référence à 'ceci' car je pourrais appeler votre fonction sur un objet différent. Vérifiez mon post détaillé sur ceci: http://stackoverflow.com/questions/1007340/javascript-function-aliasing-doesnt-seem-to-work/1162192#1162192 – SolutionYogi

+0

Parce que vous utilisez la limite de l'argument constructur, vous avez eu pour supprimer la propriété limite. Ce n'est peut-être pas une solution idéale. – SolutionYogi

0

Je ne vois pas vraiment de problèmes avec elle, mais il y a quelques choses que je ne serais probablement nettoyer.

  • Je voudrais spécifier l'outputId et limiter en tant que paramètres au constructeur. Si vous voulez toujours avoir des valeurs par défaut, vous pouvez vérifier les arguments pour undefined, ou passer un objet options.Le paramètre me dans doStuff est redondant car il est toujours identique à this et vous n'avez pas besoin de le capturer dans une fermeture.
  • Je probablement écrire doStuff comme ceci:

    var outputDiv = document.getElementById(this.outputId); 
    this.doStuff = function() { 
        this.counter++; 
        if (this.counter > this.limit) { 
         outputDiv.innerText = 'count reached limit'; 
         return false; 
        } 
        else { 
         outputDiv.innerText = 'count = ' + this.counter; 
         return true; 
        } 
    } 
    

    Il est une chose de style mineur, mais je pense qu'il est plus évident que doStuff modifie le compteur, car il est au début de la fonction au lieu de enterré à l'intérieur la clause else. En outre, déplacer le getElementById en dehors de la fonction réduit légèrement le code dupliqué.