neigedhiver
Messages postés2480Date d'inscriptionjeudi 30 novembre 2006StatutMembreDernière intervention14 janvier 201119 8 oct. 2010 à 13:47
Concernant les exceptions, rien de terrible. Au lieu d'écrire du texte avec echo, il suffit de faire ça :
throw new Exception("Message d'erreur");
C'est à l'utilisateur de gérer l'exception que tu as levée. Concrètement, pour ce cas précis, tu n'as pas "besoin" d'en savoir plus => tu pourras prendre le temps plus tard de te pencher dessus plus sérieusement, c'est pas un soucis ;)
beemoon
Messages postés1Date d'inscriptionmercredi 22 juin 2005StatutMembreDernière intervention 8 octobre 2010 8 oct. 2010 à 13:25
Merci pour toutes ces remarques très instructives. Je ne me suis pas encore aventuré sur les exceptions... chaque chose en son temps.
Le coup du easter je ne savais pas car sur MAMP sous OSX c'est présent donc je ne pensais pas que la fonction pouvait ne pas etre disponible.
Je regarde pour le reste (strtotime, DateTime...)
neigedhiver
Messages postés2480Date d'inscriptionjeudi 30 novembre 2006StatutMembreDernière intervention14 janvier 201119 5 oct. 2010 à 18:48
Salut,
Ok pour l'exercice... Je vais donc m'adonner à celui que j'aime sur ce site : critiquer ^^
Dans le constructeur, tu utilises echo si le fichier de configuration n'est pas trouvé. C'est une mauvaise chose, parce que tu ne laisses pas à l'utilisateur la possibilité de gérer lui-même et comme il l'entend les erreurs qui sont produites. Le mieux est de lever une exception. Cela aura pour effet d'interrompre l'exécution du code de la classe, donc son instanciation (ce que tu fais avec exit). L'exception sera attrapable avec un bloc try...catch et pourra être gérée comme l'utilisateur le souhaite (par exemple, enregistrer l'erreur dans un fichier de log, envoyer un mail pour prévenir l'admin, afficher une erreur qui n'indique pas d'informations sur les fichiers, etc). QUant au message d'erreur associé à l'exception, l'utilisateur sera libre d'en faire ce qu'il veut.
Au lieu de compter les arguments, tu devrais utiliser des valeurs par défaut (NULL par exemple) et tester ces valeurs. C'est un peu moins lourd et quand même plus esthétique.
On vient d'avoir un grand débat sur l'utilisation de strtotime() sur le forum. Depuis PHP5.1, le fait de ne pas préciser le fuseau horaire provoque une erreur (qu'on peut ne pas afficher avec error_reporting(0), mais c'est crade). Il faut utiliser date_default_timezone_set() pour définir le fuseau horaire que PHP utilisera (sinon, il prendra le fuseau horaire par défaut dans le php.ini, mais déclenchera quand même l'erreur).
De même, si les arguments sont incorrects, plutôt que d'écrire du texte, il est préférable de lever une exception. Il en existe même une faite exprès pour ce cas précis : BadMethodCallException. Si les arguments ne sont pas du type attendu (mauvais format de date, etc), il existe InvalidArgumentException. Ces exceptions spécialisées permettent au développeur de contrôler le plus finement possible les erreurs qui surviennent lors de l'exécution de son code.
Tu utilises, entre autres, la fonction easter_date() qui nécessite que l'extension calendar soit installée. C'est le cas sur Windows par défaut, mais pas sur Linux/Unix... Tu devrais au début de ton script vérifier que toutes les fonctions dont tu as besoin sont disponibles (avec function_exists('easter_date') par exemple, ou, plus rigoureux : extension_loaded('calendar'))
J'ai pas regardé plus en détails, ni testé. Au niveau du code, c'est propre, lisible, commenté.
Quitte à faire de l'objet, je pense qu'il peut être intéressant d'utiliser l'objet DateTime.
8 oct. 2010 à 13:47
throw new Exception("Message d'erreur");
C'est à l'utilisateur de gérer l'exception que tu as levée. Concrètement, pour ce cas précis, tu n'as pas "besoin" d'en savoir plus => tu pourras prendre le temps plus tard de te pencher dessus plus sérieusement, c'est pas un soucis ;)
8 oct. 2010 à 13:25
Le coup du easter je ne savais pas car sur MAMP sous OSX c'est présent donc je ne pensais pas que la fonction pouvait ne pas etre disponible.
Je regarde pour le reste (strtotime, DateTime...)
5 oct. 2010 à 18:48
Ok pour l'exercice... Je vais donc m'adonner à celui que j'aime sur ce site : critiquer ^^
Dans le constructeur, tu utilises echo si le fichier de configuration n'est pas trouvé. C'est une mauvaise chose, parce que tu ne laisses pas à l'utilisateur la possibilité de gérer lui-même et comme il l'entend les erreurs qui sont produites. Le mieux est de lever une exception. Cela aura pour effet d'interrompre l'exécution du code de la classe, donc son instanciation (ce que tu fais avec exit). L'exception sera attrapable avec un bloc try...catch et pourra être gérée comme l'utilisateur le souhaite (par exemple, enregistrer l'erreur dans un fichier de log, envoyer un mail pour prévenir l'admin, afficher une erreur qui n'indique pas d'informations sur les fichiers, etc). QUant au message d'erreur associé à l'exception, l'utilisateur sera libre d'en faire ce qu'il veut.
Au lieu de compter les arguments, tu devrais utiliser des valeurs par défaut (NULL par exemple) et tester ces valeurs. C'est un peu moins lourd et quand même plus esthétique.
On vient d'avoir un grand débat sur l'utilisation de strtotime() sur le forum. Depuis PHP5.1, le fait de ne pas préciser le fuseau horaire provoque une erreur (qu'on peut ne pas afficher avec error_reporting(0), mais c'est crade). Il faut utiliser date_default_timezone_set() pour définir le fuseau horaire que PHP utilisera (sinon, il prendra le fuseau horaire par défaut dans le php.ini, mais déclenchera quand même l'erreur).
De même, si les arguments sont incorrects, plutôt que d'écrire du texte, il est préférable de lever une exception. Il en existe même une faite exprès pour ce cas précis : BadMethodCallException. Si les arguments ne sont pas du type attendu (mauvais format de date, etc), il existe InvalidArgumentException. Ces exceptions spécialisées permettent au développeur de contrôler le plus finement possible les erreurs qui surviennent lors de l'exécution de son code.
Tu utilises, entre autres, la fonction easter_date() qui nécessite que l'extension calendar soit installée. C'est le cas sur Windows par défaut, mais pas sur Linux/Unix... Tu devrais au début de ton script vérifier que toutes les fonctions dont tu as besoin sont disponibles (avec function_exists('easter_date') par exemple, ou, plus rigoureux : extension_loaded('calendar'))
J'ai pas regardé plus en détails, ni testé. Au niveau du code, c'est propre, lisible, commenté.
Quitte à faire de l'objet, je pense qu'il peut être intéressant d'utiliser l'objet DateTime.
Voilà voilà.