Skip to content

Instantly share code, notes, and snippets.

@julienw
Created November 28, 2012 14:58
Show Gist options
  • Save julienw/4161795 to your computer and use it in GitHub Desktop.
Save julienw/4161795 to your computer and use it in GitHub Desktop.
code review de timerjs

j'ai juste regardé l'API pour l'instant, j'ai quelques remarques déjà là-dessus, je regarderai le code plus tard.

  • l'argument à play est pas clair. Je pense que tu peux faire une configuration fluide du style :
timer.speed(2).play();
ou même
timer.play().speed(2);
ou
timer.speed(2);
timer.play();

Suffit que toutes les méthodes retournent timer.

  • je préférerais que stop() fasse la même chose que ton play(0) (et mon speed(0) donc ;) ) Et un resume ferait la même chose qu'un play(oldSpeed) (ou speed(oldSpeed) avec ma proposition d'API)

  • pour revenir à 0, que dirais-tu d'une méthode reset() ? ton stop actuel, ce serait alors timer.stop().reset().

  • toujours sur la conf fluide, on peut gérer duration/etc comme ça aussi :

var timer = new Timer().duration(5000).delay(1000).

Et pour récupérer une valeur, au lieu d'une propriété, ce serait la même méthode :

timer.duration()
timer.delay()
  • je me dis aussi qu'on n'a pas besoin de ce new. On devrait pouvoir créer une nouvelle instance juste avec Timer():
var timer = Timer().duration(xxx);

C'est assez facile à faire si tu veux conserver la possibilité de faire le new (jette un oeil au code de jQuery), et encore plus facile si tu veux pas garder cette possibilité.

J'ai regardé un tout ptit peu le code:

  • j'enlèverais complètement ton this.set pour accéder aux variables closed. Par contre j'aime bien la manière que t'as eu de définir ces variables, ça encapsule bien leur fonctionnement.

quelques lectures sur la configuration fluide (fluent interface): http://www.martinfowler.com/bliki/FluentInterface.html et http://en.wikipedia.org/wiki/Fluent_interface :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment