Quelques remarques sur le code produit dans chorem
Salut, Gwen, j'ai regardé un peu le code que tu as fait, voici quelques remarques. Sur le code Java ================ - pas de tabulation, mettre des espaces - mettre une javadoc en haut de chaque classe pour indiquer pourquoi elle est la et ce que l'on trouve dedans - faire attention aux imports non utilisés - éviter/proscrire les méthodes statiques sauf s'il y a une bonne raison (et donc un commentaire avec), ou le nom de la classe doit etre explicite xxxUtil ou xxxHelper qui contiennent souvent des methodes statiques) - une méthode ne devrait avoir qu'un seul return (un seul point de sortie)(seul execption acceptable est en début de méthode après avoir vérifier les arguments pour sortir aussitôt s'ils ne sont pas bon. - proscrire les attributs private, mettre protected (il y a toujours un moment ou une autre personne va vouloir hériter de la classe et ne pas pouvoir faire ce qu'il veut car il n'a pas accès a toutes les info) - toutes les boucles, if, while, ... doivent utiliser { et } pour leur bloque même s'il n'y a qu'une ligne - les retours de méthode ne doivent pas montrer l'implantation utilise, car si on change l'implantation de la méthode et utilise autre chose, on est obligé de modifier la signature de la méthode (ex: Map plutôt que HashMap) - éviter de laisser trop longtemps de System.out.println dans le code, mieux vaut les transformer en log - """private void getAlerts() {""" c'est tout de même perturbant une méthode get qui retourne rien :( et private en plus :D. Préfère passer en paramètre la collection dont cette méthode à besoin plutôt qu'elle aille elle même la chercher, c'est plus testable, c'est souvent plus résistent au multithread, c'est plus réutilisation/déplaçable. - plutôt que de faire: """ if(t.getStatus().equalsIgnoreCase("scheduled")) { customClass = "ganttBlue"; } else if(t.getStatus().equalsIgnoreCase("started")) { customClass = "ganttGreen"; } else if(t.getStatus().equalsIgnoreCase("finished")) { customClass = "ganttRed"; } else if(t.getStatus().equalsIgnoreCase("closed")) { customClass = "ganttGrey"; } """ Il aurait-été mieux de faire quelque chose comme """ customClass= "gantt-" + StringUtils.toLowerCase(t.getStatus()); """ le code est plus concis, il represente mieux ce que c'est 'gantt-scheduled' plutôt que ganttBlue qui est le rôle du CSS de décider de la couleur, ça continu à fonctionner même s'il y a de nouveaux statuts (il faut seulement les prendre en compte dans le CSS rem: même avec ta méthode, il serait préférable d'écrire "scheduled".equalsIgnoreCase(t.getStatus()) car si t.getStatus retourne null ton code plante, alors que si on inverse le code retourne false (ce qui est correcte) - pourquoi faire une interface comme celle-ci ? """ public interface Extensions { public static final String[] extensions = { "Quotation", "Draft", "Sent", "Rejected", "Accepted", "Started", "Delivered", "RSV", "Warranty", "Closed", "Cancelled" }; } """ alors qu'en mettant à jour l'enum QuotationStatus tu devrais pouvoir l'utiliser et ce serait beaucoup plus propre ? Et puis le nom Extensions c'est vraiment trop générique, même si elle est dans le package project, il vaut mieux mettre des noms moins générique - un peu plus de commentaire en javadoc et dans le code, serait tout de même le bienvenu :D Utilisation de Wikitty ====================== Utiliser le select et la possibilite de faire des sommes/moyenne/max/min Par exemple dans: """ WikittyQuery expenseQuery = new WikittyQueryMaker() .and()....end(); WikittyQueryResult<FinancialTransaction> result = client.findAllByQuery(FinancialTransaction.class, expenseQuery); double total = 0; for(FinancialTransaction ft : result.getAll()) { total += ft.getAmount(); } """ On pourrait avoir directement le résultat au lieu de devoir itérer sur le résultat avec un "Select sum(FinancialTransaction.amount) ..." L'intérêt est de laisser wikitty faire le boulot, ce qui évite de faire transiter N objet entre le serveur et le client alors que la seul chose qui nous intéresse est une somme. Si tu avais le temps de corriger au moins les tabulations, ce serait parfait. Si tu as le temps de modifier plus de chose, c'est encore mieux :D -- Benjamin POUSSIN -------------------- tél: +33 (0) 2 40 50 29 28 email: poussin@codelutin.com http://www.codelutin.com
participants (1)
-
Benjamin POUSSIN