aurel2108/Freeze — pull request #2 by aumgn (2012-06-02 09:03:59+00:00)
Title: Diverses améliorations

Yop,
Suite a mon message sur bukkit.fr, je m suis dit qu'il serait plus simple de montrer directement qu'elle serait les points à améliorer avec un PR que de rentrer dans des longs discours.
Rien n'est testé, les modifications ne sont pas prévu pour être pullé mais juste pour te donner des pistes que tu pourra reprendre de ton coté ;).

Quelques commentaire par commit.
- Extract and clean up commands
J'ai extrait les commandes dans une class FreezeCommands pour plus de lisibilité.
J'ai aussi changé pas mal de trucs. Par exemple, j'ai remplacé l'utilisation de TimeTask et ci. par celle du Scheduler de Bukkit qu'il est largement préférable d'utiliser.

- Add .gitignore
Les dossiers bin, .classpath, .project ne font pas partie du code source. En plus ce sont des fichiers qui sont géré par eclipse et peuvent changer automatiquement ce qui est plutôt génant pour la lisibilité des commits.
Du coup j'ai ajouté le .gitignore et je les ais supprimé du dépot pour qu'il ne soit plus pris en compte.

- Use sets rather than list
Comme dit dans mon message précédent. Un HashSet est plus approprié. L'utilisation est très proche d'une liste. (Les deux classes implémentent l'interface Collections qui définit les méthodes contains(), remove(), add(), etc). La différence entre les deux c'est juste l'implémentation. Une liste ajoute les éléments les uns après les autres. Un HashSet utilise la fonction hashCode() pour repartir les éléments.
Le gros avantage du HashSet en l'occurence c'est que la méthode contains s’exécute dans une durée temps constante quelques soit le nombre d'éléments, alors que celle de ArrayList va être proportionnelle à ce nombre d'éléments. (Plus il y a d'élément plus la recherche va durer longtemps.

- Use JavaPlugin#getLogger() rather than a custom logger
Déjà expliqué =), j'ai aussi changé certains appels à System.out.println utilisées pour reporter des erreurs en utilisant la méthode Logger#log(Level, String, Exception) plus propre.

- Remove unneeded variables and rename FreezePlayerListener to FreezeListener
Le renommage est pas très important mais nommé un Listener en tant que PlayerListener, BlockListener, etc.. est un vestige de l'ancien système d'événement qui est maintenant dépassé pa rle nouveau beaucoup plus flexible =).
Le reste je l'avais déjà survolé dans le poste sur Bukkit.fr, j'ai enlevé les variables d'instances inutiles et j'ai remplacé certains bout de code par des appels à des méthodes de Bukkit équivalentes.

- Delegate frozen players management to Freeze class
Ca c'est le gros morceaux, l'idée c'est d'isoler (on pourrait même dire de cacher) toute la logique de gestion des joueurs frozen (est-ce qu'il est temporairement frozen, est-ce qu'il a la permission frozen.never etc...) dans la classe principale et derrière quelques méthodes bien nommées pour rendre le code plus lisible et plus simple à modifier. J'ai du coup aussi enlevé les variables statiques.
L'interêt, en plus de la lisisbilité, c'est par exemple que si tu as besoin d'ajouter une condition pour empêcher qu'un joueur puissent être frozen, tu aura juste à modifier la méthode canBeFrozen(Player) au lieu de changer toutes les conditions dans les commandes et les méthodes du listener.

Voilà hésites pas à commenter si il y a des trucs qui te chagrine ;).