Salut !

@Bunyan

Désole pour le retard, j'ai du m'y reprendre à plusieurs fois pour répondre à tout !
Allez, c'est parti pour le retour du retour :D :

"Déclaration de TOUTES les collections par des types concrets, non par des interfaces."

Ca c'est un peu fait exprès, par souci d'optimisation, pour éviter le plus possible de liaisons dynamiques (mais j'avoue que je sais pas bien si c'est efficace, j'ai pas fait de test).

"Les collections n’ont pas de types génériques, ce qui implique de devoir obligatoirement passer par tout plein de instanceof pour ne pas se tromper dans l’ajout et le retrait."

Exact, maintenant que les sources sont en 1.5, je vais rajouter la généricité, je le note ;) . Cela dit, il n'y a aucun instanceof car on sait toujours quels types sont dans les listes...

"Beaucoup de « magic number » => préférer des constantes nommées plutôt."

Exact, je le note ;) .

"Truc mineurs : a = a * norm => a *= norm. C’est pareil, plus court et souvent plus lisible (attention à ne pas en abuser)."

Exact, erreurs de jeunesse ;) , je vais leurs faire la chasse...

"Des assignements à l’indice d’une boucle « for ». Pas fan personnellement, je préfère éviter le plus possible, ça montre un souci d’algo, mais de temps en temps, c’est inévitable pour faire quelque chose de « court ». Peut-être passer par un itérateur, plutôt ?"

Tu as raison, en général je faisais ça quand il n'y a qu'une seule boucle dans l'algo, mais c'est vrai que c'est moins propre, je ne note ;) .

"Assignation d’une valeur à un paramètre de méthode. Ca, c’est personnel et dépend des équipes. Pour moi, les paramètres sont des constantes intouchables (dans le cas de type primitif), appel de méthode possibles."

C'est vrai qu'il faut y faire gaffe, mais perso je n'y vois pas trop d'inconvénient si c'est maitrisé ;) . Je fais ça uniquement dans le cas d'algos "définitifs".

"Se renseigner sur la classe « MouseAdapter », et plutôt l’utiliser que « MouseListener » dans les cas qui correspondent (typiquement, pour toi : permet de définir 2 méthode au lieu de faire 6 méthodes dont 4 vides)."

Ca c'est aussi pour une question de performances : les adapters sont des surcouches des listeners donc je préfère utiliser directement ceux-ci.

"Ne pas hésiter à créer une classe réelle (interne ou non) au lieu d’une classe interne anonyme dans les cas de classe anonyme à forte complexité."

Ok ;) , c'est vrai que ça serait plus propre, je le note ;) .

"Il te manque pas mal d’annotation « @Override ». En soit, elle n’importe pas grand-chose (à ma connaissance), mais permettent de savoir directement que la méthode est redéfinie de quelque chose. Utile donc."

Exact, j'avais la flemme de les mettre, mais je vais le faire ;) .

"Des soucis avec « clone ».
• La première instruction doit plutôt être super.clone(), histoire de construire toute l’arborescence dans le cas de construction d’une classe fille d’un cloneable. Ce n’est pas obligatoire, juste que ne pas le mettre peut mener à des comportements étranges et difficilement compréhensibles.
• Beaucoup de méthodes « clone » dans des classes qui n’implémentent pas « cloneable », ce qui fait bizarre. L’implémentation de cloneable permet d’avoir une indication claire de la méthode, et de ne pas être au p’tit bonheur la chance."

Exact, je m'emmêle les pinceaux avec clone(), il faut que je mette ça au clair, je le note ;) .

"Il reste des TODO (majoritairement générés par Eclipse)."

Yes, je le note ;) .

"Utilisation de Vector non générique. Franchement, remplace par des List à la déclaration, rend les génériques et tente de remplacer tes Vector par autre chose (ArrayList ou un autre conteneur plus adapté aux besoin)."

Oui, je vais rajouter la généricité, mais les Vector semblent être les types les + rapides ;) .

"Tu déclares tes tableaux comme en C :p
En Java, les crochets sont plutôt sur le type que sur la variable. Ca ne change strictement rien."

Exact, je le note, c'est un coup comme ci un coup comme ça, je vais "unifier" ça ;) .

"Pour la méthode « equals » avec des String, je te conseils très fortement d’invoquer la méthode sur la constante. Tu n’as ainsi pas besoin de faire de test de nullité avant. De plus, je te conseils aussi très fortement de mettre tes String constantes en static final, pas simplement en dur dans le code. Si elles sont utilisées à plusieurs endroits, ça permet d’éviter les erreurs de type, et quand il faut changer/renommer, c’est BEAUCOUP plus rapide."

Hey, j'avais pas réfléchi à ça sur "equals()" merci ! je le note ;) .

"Pour tester si une collection est vide, tu as la méthode isEmpty() généralement. Pas la peine de faire if (maCollec.size() == 0)."

Ha ha, c'est clair, je savais même plus que j'avais fait ça !

"Concernant le « this », c’est une question de point de vue : pour moi, ne pas le mettre quand ce n’est pas nécessaire. Pour d’autre, le mettre tout le temps. Dans certaines classes, tu l’utilises de manière inutile, mais ça ne change strictement rien, si ce n’est l’esthétique."

Exact, en fait je vais les enlever, je le note ;) .

"Tu as beaucoup de parenthèses inutile pour le code, mais qui semble aider à la lecture (je n’ai pas tout regardé, mon inspection me retourne 643 de ces cas-là)."

Oui, c'est pour la lisibilité, ça m'aide à m'y retrouver, par exemple je ne sais jamais si la multiplication est prioritaire sur la division ou l'inverse :D ! (je crois que c'est l'inverse ;p )

"Je te conseils aussi de t’intéresser au for each, qui est très pratique "

Exact, pour tout ce qui est parcour de listes, je le note ;) .

"Tu as la branche « else » de ta méthode « process » dans la classe JGL_Motion_Slide qui ne sert à rien (ligne 113). Déroule le code, tu devrais comprendre."

Hmm, là je ne vois pas... Ca dit qu'il y a collision si au moins un impact est détecté dans la séquence de "glissement"... A priori ça marche bien car cette méthode retourne la bonne valeur...

"Tu as pas mal d’assignation après test de nullité. C’est une question de goût, mais tu pourrais mettre un opérateur ternaire ici. Attention, ceux-ci rendent facilement le code illisible."

Oui, c'est vrai. Du coup je vais peut-être laisser comme ça car effecivement je ne suis pas fan de ces opérateurs (peut-être parce que pas l'habitude ;) ).

"Classe « JGL_Motion_Bounce », ligne 71-72. Tu fais un loops = 0 puis un while (loops == 0) => tu peux remplacer par un « while(true) ». Vu que je ne sais pas ce que tu veux faire ici, concrétement, ce n’est pas forcément une bonne idée de faire une boucle pseudo-infinie. Vérifie bien que tu en sortiras obligatoirement avec le code de sortie que tu as prévue (si ce n’est pas déjà fait)."

Exact, ça c'est de l'étourderie, je le note ;) .

"Selon les conventions Java, les variables statiques sont plutôt mises avec CE_TYPE_DE_NOMENCLATURE. De plus, tu as pas mal de variable statique que tu considères comme des constantes qui ne sont pas déclarées en tant que telle (il manque le mot-clef « final »)."

Ok, je vais faire la chasse aux "final", cela dit ces variables statiques sont souvent dans des classes final, crois-tu que c'est la peine de le rajouter dans ce cas ?
Pour la nomenclature, je ne sais pas trop si dans de nombreux cas ça n'alourdirais pas le code... C'est vrai que j'ai certaines classes peu orthodoxes pour du Java qui se rapprochent plus du procédural pour la performance...

"Les trucs comme « if (resultat == true), ça ne sert à rien
Autant remplacer par « if (resultat) », non ?"

Ha ha, si ! Ca c'est des fautes de fatigue, ok je le note ;) .

"Pour ton switch de JGL_3DMatrix, je te conseils de mettre les valeurs en constantes, plutôt que de simple valeurs avec un commentaire non-javadoc à côté."

Ok ;) , ça je crois que ça devait rester temporaire, et je l'ai oublié :p !

"Tu as des switch sans branche « default » (classes JGL_ImageLoader et JGL_ImageScaler), vérifie bien que tu es obligatoirement dans une des branches, et que c’est strictement impossible sinon.
Sinon, rajoute plutôt un case « default »."

Exact, c'est vrai que ça serait plus prudent, je vais le rajouter ;) .

"Tu as des « return ; » à la fin de méthode sensée retourner void. Ca ne sert à rien (classes JGL_Data3D et JGL_ReaderObj)."

Ha ha, exact ! Je saurais même pas dire pourquoi j'ai mis ça :D .

"Ta gestion des Exceptions est calamiteuse. Ne fais JAMAIS de « catch (Exception) » ni de « throw new Exception() ». Utilise TOUJOURS des exceptions qui ont un véritables sens."

Tu as raison, j'ai toujours eu la flemme, depuis le départ, de faire un bon système d'exceptions, je vais reprendre ça entièrement ;) .

"Mon inspection me dit que ta méthode « addBone » retourne absolument toujours « true ». Je ne sais pas si c’est vraiment le cas."

Hmm, à priori ce n'est pas le cas, sinon elle ne pourrait pas marcher, mais tu me fais douter :p , je vais tester ça. Avec ce type de méthode récursive, une erreur est vite arrivée !

"La méthode « getMaterials » dans JGL_ReaderObj ne lance jamais d’exception Exception."

Hé hé, exact, surement le résultat d'un malheureux copier-coller d'une autre méthode :p .

"Tu fais des accès directs à des attributs déclarés en tant que « private », pas une bonne pratique du tout. Passe plutôt par des accesseurs."

Tu as raison, mais c'est pourtant bien pratique (surtout dans les méthodes de clonage), bon je vais limiter ça au max ;) .

"Tu as des visibilités « package » (i.e. : absence de modificateur de visibilité). Personnellement, je fais en sorte de ne pas en avoir, mais c’est aussi un peu à la discrétion."

Oui, j'ai fait ça quand j'étais sûr qu'une classe n'avait pas à être accédée depuis un autre package, dans le cas d'une sorte de "Facade" où une seule classe sert d'interface au package et les autres sont utilisées derrière.

"Pour un bon principe d’encapsulation, mets tout tes attributs en « private » et utilise des accesseurs (getter/setter). Cela permet de bien délimiter les responsabilités de chacun, et de pouvoir facilement limiter les effets de bord (si c’est en lecture seule, le getter retourne une version non-modifiable de l’objet, par exemple)."

Oui, je respecte l'encapsulation autant que possible, mais il y a des structures de donnée "critiques" comme les objets géométriques (vecteurs, plans, BSPs, etc...) utilisées 1000000 de fois par frame sur lesquelles le moindre ralentissement se ressent fortement :p .

"J’ai tout plein de « uncheckedWarning » lié a l’absence de test avant de retirer d’une liste et de transtyper."

Ca devrait être réglé par la généricité je crois...

"Tu as un constructeur publique dans la classe JGL_Loop, sauf que celle-ci est abstraite, donc au mieux doit en avoir un en protected."

Eh oui, j'avais pas réfléchi à ça ! Je le note ;) .

"Tu fais un tout petit peu d’auto—(un)boxing inutile."

Euh, je sais pas ce que c'est :p (le boulet) !

"Tu fais des copies de tableau « à la main ». Utilise plutôt System.arrayCopy
Pareil pour tableau vers collection. Utilise Collections.addAll "

Exact, ces copies à la main datent d'il y a looongtemps ! Je le note ;) .

Ouf, voilou, je te remercie encore pour ce retour, et j'y retourne pour arranger tout ça !

@godrik

"Note qu'en C++ (je sais que tu fais du java), *= est souvent plus rapide que = et * pour les types complexes."

Hmm, c'est bon à savoir ! Cela dit ça ne concerne pas Java car on ne peut pas surcharger les opérateurs .

@[-ArK-]

Eh bien moi je suis plutôt d'accord avec toi pour les return ;) . Un seul point de sortie peut devenir vraiment galère voire ingérable par exemple pour des fonctions récursives complexes comme celles des BSP dans mon cas...

" "Déclaration de TOUTES les collections par des types concrets, non par des interfaces."

Ca c'est un peu fait exprès, par souci d'optimisation, pour éviter le plus possible de liaisons dynamiques (mais j'avoue que je sais pas bien si c'est efficace, j'ai pas fait de test)."

Ca n'optimise strictement rien. Ca fait juste que le code est plus difficile à changer.
Je n'ai strictement jamais vu de gain de perf notable (ou non) par ce type de pratique. Si tu as une source, je serai intéressé.
Pour plus de flexibilité et de lisibilité, je te conseils toujours de les déclarer en tant que List.

" "Se renseigner sur la classe « MouseAdapter », et plutôt l’utiliser que « MouseListener » dans les cas qui correspondent (typiquement, pour toi : permet de définir 2 méthode au lieu de faire 6 méthodes dont 4 vides)."

Ca c'est aussi pour une question de performances : les adapters sont des surcouches des listeners donc je préfère utiliser directement ceux-ci. "

Même remarque qu'au-dessus.

" "Il te manque pas mal d’annotation « @Override ». En soit, elle n’importe pas grand-chose (à ma connaissance), mais permettent de savoir directement que la méthode est redéfinie de quelque chose. Utile donc."

Exact, j'avais la flemme de les mettre, mais je vais le faire ;) . "

Eclipse le fait tout seul (en fait ... n'importe quel IDE un peu évolué le fait tout seul).

" "Tu as la branche « else » de ta méthode « process » dans la classe JGL_Motion_Slide qui ne sert à rien (ligne 113). Déroule le code, tu devrais comprendre."

Hmm, là je ne vois pas... Ca dit qu'il y a collision si au moins un impact est détecté dans la séquence de "glissement"... A priori ça marche bien car cette méthode retourne la bonne valeur... "

Tu fais un "return" dans ton "if", donc le else est inutile.

Si tu fais :
if (machin <5)
{
...
...
...
return uneOpe;
}
else
qqch;

instruc1;
instruc2;
instruc3;
return uneAutreOpe;

Dans ce cas, le "else" est tout à fait inutile, puisqu'il sera obligatoirement fait, ainsi que les instructions le suivant.
Par contre, si tu n'as pas de "return" dans le if, le "elfe" à de l'importance.

" "Selon les conventions Java, les variables statiques sont plutôt mises avec CE_TYPE_DE_NOMENCLATURE. De plus, tu as pas mal de variable statique que tu considères comme des constantes qui ne sont pas déclarées en tant que telle (il manque le mot-clef « final »)."

Ok, je vais faire la chasse aux "final", cela dit ces variables statiques sont souvent dans des classes final, crois-tu que c'est la peine de le rajouter dans ce cas ?
Pour la nomenclature, je ne sais pas trop si dans de nombreux cas ça n'alourdirais pas le code... C'est vrai que j'ai certaines classes peu orthodoxes pour du Java qui se rapprochent plus du procédural pour la performance... "

"final" sur une classe implique uniquement que celle-ci ne peut pas avoir de fille (de classes héritant d'elle). Absolument tout son contenu est toujours modifiable sinon.
Donc oui, c'est utile, puisque ce ne sont pas des constantes que tu manipules, mais des variables globales pouvant être modifiées
Pour le second point, nomenclature et performance, honntement, je pense que ça n'a rien à voir. La seule influence que ça peut avoir est vis-a-vis de la taille du fichier, donc le nombre de caractère utilisé. Mais bon, si ceci importe vraiment, autant passer par un obfuscateur qui inline intégralement le code.
Si je suis à côté, je ne comprends absolument pas ce que tu veux dire en mettant "nomenclature" et "procédural" à côté. Pour moi, c'est comme mettre "vache" et "ciel", ça n'a rien à voir.

" "Les collections n’ont pas de types génériques, ce qui implique de devoir obligatoirement passer par tout plein de instanceof pour ne pas se tromper dans l’ajout et le retrait."

Exact, maintenant que les sources sont en 1.5, je vais rajouter la généricité, je le note ;) . Cela dit, il n'y a aucun instanceof car on sait toujours quels types sont dans les listes... "

Toi, oui, tu sais ce qu'il y a dedans. Ton code, lui, ne le sait pas. Il n'est absolument pas sécurisé de ce côté-ci. Quelqu'un qui reprend les sources et rajoutes un String dans un de tes Vector (par exemple) arrivera à le faire, et fera tout planter avec une ClassCastException.

Je ne sais absolument pas comment tu as fait tests, mais je te le certifie, la classe Vector est l'une des collections les plus lentes.
C'est l'une des rares a être synchronisées, donc elle perd du temps en vérifiant cette synhcro.

Tu peux rechercher des benchmarks sur le net, tu ne devrais normalement en trouver aucun en faveur de Vector (sauf en environnement multi-thread).
Ne pas oublier le compilateur JIT, qui impose de devoir faire la même manipulation X fois le temps que celui-ci "chauffe". J'ai vu des bench qui mettaient Vector gagnant sur la première passe, mais loin derrière (facteur temps x2) par rapport à une ArrayList.

De plus, si tu accèdes uniquement aux premiers/derniers et ne fait que des opérations d'ajout/suppression (pas de "prendre le i-ème"), une LinkedList devrait normalement être plus rapide.

" "Tu fais des accès directs à des attributs déclarés en tant que « private », pas une bonne pratique du tout. Passe plutôt par des accesseurs."

Tu as raison, mais c'est pourtant bien pratique (surtout dans les méthodes de clonage), bon je vais limiter ça au max ;) . "

Pratique ? Mouais ... je ne vois strictement aucune différence entre maClasse.x; et maClasse.getX(); en terme d'écriture.
Pour les méthodes de clonage, vu que tu es dans l'objet, tu as accès directement aux attributs, donc passer par des accesseurs n'impactent normalement pas ça. Si tu parles, par contre, de faire des suites comme :
newObject.x = oldObject.x;
newObject.y = oldObject.y;
newObject.z = oldObject.z;
newObject.a = oldObject.a;

Je te conseillerai très fortement de créer un constructeur de copie. Ca évitera ce type de chose.
Faudra que je regarde si c'est bien ça qui causait ces inspections.

"
"Tu fais un tout petit peu d’auto—(un)boxing inutile."

Euh, je sais pas ce que c'est :p (le boulet) ! "

Y'a pas de mal.

Tu fais (de mémoire), des choses comme int unInt = Integer.valueOf(uneString).intValue();
Le 'intValue()' ici est inutile.

"Pour plus de flexibilité et de lisibilité, je te conseils toujours de les déclarer en tant que List."

Bon, je te crois, je m'étais basé sur mon simple bon sens, mais je vais quand même faire quelques tests ;) .

"Tu fais un "return" dans ton "if", donc le else est inutile.

Si tu fais :
if (machin <5)
{
...
...
...
return uneOpe;
}
else
qqch;

instruc1;
instruc2;
instruc3;
return uneAutreOpe;

Dans ce cas, le "else" est tout à fait inutile, puisqu'il sera obligatoirement fait, ainsi que les instructions le suivant.
Par contre, si tu n'as pas de "return" dans le if, le "elfe" à de l'importance. "

Oui, mais le tout est dans un "while" et le résultat de la condition est fonction du déroulement de la boucle ;) .

""final" sur une classe implique uniquement que celle-ci ne peut pas avoir de fille (de classes héritant d'elle). Absolument tout son contenu est toujours modifiable sinon.
Donc oui, c'est utile, puisque ce ne sont pas des constantes que tu manipules, mais des variables globales pouvant être modifiées"

Ok, bien vu, je vais modifier ça ;) .

"Pour le second point, nomenclature et performance, honntement, je pense que ça n'a rien à voir."

Non non, c'est pas ce que je voulais dire ! Je voulais parler de la lisibilité qui serait peut-être amoindrie si j'adopte cette nomenclature car j'ai parfois beaucoup de ces objets, utilisés frequemment, et ça serait lourd de les noter comme ça ;) .

"Je ne sais absolument pas comment tu as fait tests, mais je te le certifie, la classe Vector est l'une des collections les plus lentes.
C'est l'une des rares a être synchronisées, donc elle perd du temps en vérifiant cette synhcro."

Ha ha, en fait je crois que j'avais ajouté et retiré un objet 1000000 de fois. Du coup ça rejoint ta remarque sur la 1ere passe ! Bon, tu m'as convaincu, je vais mettre des ArrayList ;) .

"Je te conseillerai très fortement de créer un constructeur de copie. Ca évitera ce type de chose.
Faudra que je regarde si c'est bien ça qui causait ces inspections."

Oui, c'est ce que j'ai fait dans la plupart des cas, mais c'est compliqué pour les types récursifs... Mais c'est vrai que ça serait plus propre, je vais y bosser ;) .

"Tu fais (de mémoire), des choses comme int unInt = Integer.valueOf(uneString).intValue();
Le 'intValue()' ici est inutile."

Ha ha ! Ok, je vais virer ça !

Mazette, encore du boulot !!

C'est du très beau boulot, et merci à Bunyan d'apporter son point de vue sur son code c'est super intéressant de lire les échanges. Bravo à toi N_I_C_S !

Note à part : j'ai un style écrit un peu agressif, et je n'arrive pas vraiment à le corriger. Mes excuses d'avance si certaines choses sont mal prises.

Je vais continuer de me renseigner sur les différents comparatif vector/arralist/autre ... j'ai vu des choses qui commencent à me faire douter :/

@Paulop

Merci beaucoup ! Et c'est vrai que c'est précieux d'avoir un avis comme celui-là, moi qui ne connais pas toutes les subtilités du langage...

@Bunyan

Hey, no soucy ! Tu dis les choses direct et c'est très bien comme ça ;) .
Ok pour les Vector, j'attend ton avis :D .