lesckrank
Messages postés19Date d'inscriptiondimanche 24 août 2014StatutMembreDernière intervention27 août 2014
-
24 août 2014 à 12:36
lesckrank
Messages postés19Date d'inscriptiondimanche 24 août 2014StatutMembreDernière intervention27 août 2014
-
24 août 2014 à 19:41
Cette discussion concerne un article du site. Pour la consulter dans son contexte d'origine, cliquez sur le lien ci-dessous.
lesckrank
Messages postés19Date d'inscriptiondimanche 24 août 2014StatutMembreDernière intervention27 août 2014 24 août 2014 à 19:41
J'ai revu le code en prenant en compte ce que tu m'as dis ...
Et j'ai également posté le Writer.java ;)
lesckrank
Messages postés19Date d'inscriptiondimanche 24 août 2014StatutMembreDernière intervention27 août 2014 24 août 2014 à 17:52
Merci pour tous ces conseils ! Je vais essayer de revoir mon code avec ce que tu m'as dis et je poste le Writer.java juste après.
KX
Messages postés16733Date d'inscriptionsamedi 31 mai 2008StatutModérateurDernière intervention31 janvier 2024127 24 août 2014 à 15:24
"N'hésitez pas à me dire ce que vous en pensez !" Programmer en Java, c'est à dire orienté objet, ce n'est pas faire des copier coller !
Tu as 16 méthodes qui font sensiblement la même chose, et ton code c'est grosso modo 16 copier coller du même code ! Il aurait fallu factoriser tout ça...
Imagine que tu veuilles changer un morceau de code, par exemple changer le e.printStackTrace() pour les mettre dans un Logger, et bien tu devrais changer ça 32 fois dans ton code, alors qu'une ou deux fois pourrait suffire !
Le truc très simple, comme tu as
ecrire(String path, String text, boolean replace)
, alors pour les suivants tu aurais juste à faire :
public static void ecrire(String path, int text, boolean replace)
{
ecrire(path, String.valueOf(text), replace);
}
C'est quand même nettement mieux que de se retaper le même code deux fois !
Dans la catégorie code inutile ton if/else pour gérer le boolean replace est là encore très inutile.
Au lieu d'avoir :
if (!replace)
... new FileWriter(path, true)
else
... new FileWriter(path, false)
Tu devrais directement faire :
... new FileWriter(path, !replace)
Ça évite d'avoir encore un copier coller entre le if et le else juste pour un booléen qui change de valeur... et je ne parle même pas des 2 if/else dans les codes à tableaux !
D'ailleurs avec les tableaux tu fais :
int i = 0;
do {
ecri.println(text[i]);
i++;
}while (i != text.length);
Très clairement cette boucle devrait être remplacée par un for :
for (int i = 0; i != text.length; i++)
ecri.println(text[i]);
Au passage :
- Tu n'as pas besoin de faire un flush() avant un close()
- Il ne faut pas masquer les exceptions de ton try/catch par un System.out.println. Il serait plus propre que tes méthodes throws les IOException, charge à l'utilisateur de les récupérer pour régler le problème.
- Dans les types primitifs tu as oublié les boolean, et pour être complet il faudrait aussi avoir une méthode pour les Object
- Les noms de méthodes devraient être en anglais, exit donc les "ecrire"
Bref, ton code est très naïf dans sa conception et dans l'absolu il ne sert pas à grand chose. Je mets 2 étoiles parce que ça devrait fonctionner (même si je n'ai pas testé), mais il y a quasiment tout à revoir !
Sinon, comme le disait NHenry, postes ton fichier Writer.java !
Moi je lis directement le code dans le .class mais tout le monde ne sait pas faire...
NHenry
Messages postés15102Date d'inscriptionvendredi 14 mars 2003StatutModérateurDernière intervention27 mars 2024159 24 août 2014 à 13:29
24 août 2014 à 19:41
Et j'ai également posté le Writer.java ;)
24 août 2014 à 17:52
24 août 2014 à 15:24
Programmer en Java, c'est à dire orienté objet, ce n'est pas faire des copier coller !
Tu as 16 méthodes qui font sensiblement la même chose, et ton code c'est grosso modo 16 copier coller du même code ! Il aurait fallu factoriser tout ça...
Imagine que tu veuilles changer un morceau de code, par exemple changer le e.printStackTrace() pour les mettre dans un Logger, et bien tu devrais changer ça 32 fois dans ton code, alors qu'une ou deux fois pourrait suffire !
Le truc très simple, comme tu as , alors pour les suivants tu aurais juste à faire :
C'est quand même nettement mieux que de se retaper le même code deux fois !
Dans la catégorie code inutile ton if/else pour gérer le boolean replace est là encore très inutile.
Au lieu d'avoir :
Tu devrais directement faire :
Ça évite d'avoir encore un copier coller entre le if et le else juste pour un booléen qui change de valeur... et je ne parle même pas des 2 if/else dans les codes à tableaux !
D'ailleurs avec les tableaux tu fais :
Très clairement cette boucle devrait être remplacée par un for :
Au passage :
- Tu n'as pas besoin de faire un flush() avant un close()
- Il ne faut pas masquer les exceptions de ton try/catch par un System.out.println. Il serait plus propre que tes méthodes throws les IOException, charge à l'utilisateur de les récupérer pour régler le problème.
- Dans les types primitifs tu as oublié les boolean, et pour être complet il faudrait aussi avoir une méthode pour les Object
- Les noms de méthodes devraient être en anglais, exit donc les "ecrire"
Bref, ton code est très naïf dans sa conception et dans l'absolu il ne sert pas à grand chose. Je mets 2 étoiles parce que ça devrait fonctionner (même si je n'ai pas testé), mais il y a quasiment tout à revoir !
Sinon, comme le disait NHenry, postes ton fichier Writer.java !
Moi je lis directement le code dans le .class mais tout le monde ne sait pas faire...
24 août 2014 à 13:29