Bonjour à tous,
Je développe une appli J2EE en ce moment et j'ai un problème que j'ai du mal à identifier réellement et à résoudre.
 
Il se trouve qu'en testant l'appli tout marche bien et tout à coup j'ai un NullPointerException qui pète au niveau de mes classes métiers. Ca me semble d'autant plus bizarre que la fois d'avant tout c'est bien passé (même cas exactement, aucun changement).
 
J'ai pensé à un problème de performance de mon code mais j'avoue que ce n'est pas mon fort.
 
voici l'exception :

 
La méthode getInstance de ma classe métier Product renvoie une instance d'un produit selon le numéro (Id) passé en argument.
 
est-ce que quelqu'un aurait une idée pour savoir d'où ça pourrait venir ?
 
 
edit : après un certain temps j'ai donc pu constater qu'il s'agissait en fait d'une SQLException disant "Cannot get a connection, pool exhausted".
 
maintenant je ne comprends pas pourquoi, il me semble que je close bien tous mes objets

Beh, ajoute du debugging sur la ligne en question, ou passe justement en mode debugger pour voir ce qui se passe.
 
NPE, ça peut pas être 36 trucs différents, hein.

Je vais rajouter que l'exception ne pète pas toujours pour la même méthode. ça peut etre le getInstance de la classe Company ou le getAllInstances.
Et une fois que j'ai eu une de ces erreurs, plus rien ne fonctionne. A chaque fois que je fais précédent et que je clique sur un lien j'ai un NullPointerException.
 
C'est pour ça que je ne pense pas que le problème vienne vraiment de mon code, mais peut etre plus du paramétrage de tomcat ou un truc du genre

j'y ai pensé mais comme je l'ai dit après, j'ai toujours un NPE mais pas toujours sur la même méthode ni sur la même classe...
Alors qu'à d'autres moments ça marche très bien...
 
pourquoi un NPE pèterait qu'à certains moment et pas d'autres ?

Là c'est d'autant plus bizarre. Il vient de me lever la NPE sur cette ligne en gras :
 

 
Un NPE peut péter sur un close du resultset ? c'est bizarre quand même.

Parce qu'à certains moments tu accèdes à une référence qui est "null" ?
 

 
Et c'est précisemment ce que des traces supplémentaires de debugging ou un debugger pourront t'apprendre.

 
C'est évident !
 
Si ton Result rs = ... pète, tu essaye ensuite d'y accéder alors qu'il est NULL !
 
Toujours tester préalablement :
 

Pour le surplus, ta gestion des exceptions n'est pas correcte.
 
Ton rs.close() et tout les autres "cleanups" devraient se trouver dans le bloc finally.

ah ok merci pour ça
 
je vais essayer de mieux débugguer le truc, mais j'ai du mal à voir pourquoi il perd la référence à certains moment
surtout qu'à un moment donné le NPE a pété sur mon Company.getIntance et j'ai vu que le companyNum était bien en argument...

tu veux dire de plutot faire ça :
 

 
mais mon rs.close() était déjà dans le bloc finally !

Ton code est sans doute un peu sloppy si j'en juge l'échantillon que tu as présenté. Revois aussi la gestion des exceptions... Tu as bien compris ma remarque sur le cleanup dans le fianlly ?

ah bah tant que tu y est dis moi toutes les critiques que tu trouves à dire sur mon code
 
si ça peut m'aider à mieux programmer chui pour !
 
edit : oui je comprends ta remarque mais mon rs.close était déjà dans le finally non ? et là je ne vois pas quoi d'autre à rajouter dedans

Voilà, c'est bcp mieux.    
 
Fais gaffe avec ton bloc catch : tu ne relances pas l'exception. Ca veut dire que l'appelant recevra, in casu, "list" vide. Comment faire la différence entre une liste réelement vide (parce que rs ne contient rien) et le cas "erreur, shit happened" ? Ca peut être gênant (mais dans certains cas, c'est +/- acceptable). Peut-être devrais-tu relancer l'exception, ce qui permettra in fine l'affichage d'un message d'erreur.

je ne comprends pas bien ce que tu veux dire.
Tu veux dire que je ne fais pas la différence entre un rs vide et une erreur dans mon rs ?
Alors comment je devrais faire apparaître cette différence ?
 
sinon est-ce que c'est mieux de faire mon  

en mettant un throws SQLException ou de faire ça :

c'est pas ça que j'aurais conseillé moi...
 
Son cas de NPE sur rs.close() ne peut s'expliquer que si QueryBroker.getInstance().executeQuery() retourne null => il faudrait investiguer sur les cas où cette méthode retourne null (est ce que c'est un cas normal ? décrit dans la doc ?)
 
si le fait de retourner null est un cas prévu, il serait préférable de le tester juste après (avant même de rentrer dans le bloc try de traitement du resultset).
 
si ce n'est pas prévu, c'est surement un bug dans la classe QueryBroker ... c'est là qu'il faut corriger. D'ailleur, ca ne me parait pas normal qu'une méthode de ce type ne puisse pas balancer d'exception ... doit y avoir un catch pas jojo là dedans ...

Dans ton block "catch", tu loggues bien ton exception de manière adéquate, et tu la relances :

ou encore

De cette manière, l'appelant se prend la patate chaude, à charge pour lui soit traiter l'exception, soit de la relancer.
 

 
C'est sans doute mieux, car en cas de SQLException sur le close, il n'y a sans doute rien grand-chose à faire, ni de la part de l'appelant, ni dans la méthode courante. Renvoyer cette exception à l'appelant n'est sans doute pas indiqué. Par contre, au minimum, dans le catch, je laisserais une trace.
 
Ce n'est qu'à moitié satisfaisant, et seul un mécanisme plus robuste renvoyant à la fois le résultat et d'éventuels warnings/errors/infos permet une gestion fine de tous ces cas...

Ah c'est possible. La classe QueryBroker je l'ai récupéré d'un projet d'école, mais je n'était pas moi qui l'ai faite.
 
voici la méthode executeQuery :

la variable privée _ds est un DataSource initialisé à null.
et le getInstance renvoie une nouvelle intance du QueryBroker. La création d'une instance se fait comme ceci :

 
est-ce que tu vois quelque chose de bizarre la dedans ?

bha oui. Les exceptions sont gérées n'importe comment => tu te retrouves avec des return de variables non affectées => NPE.
 
erreur classique
 
Essaye de suivre les conseils qui viennent de t'être donnés
Tu trouveras d'autres conseils sur la gestion des exceptions là : http://forum.hardware.fr/hardwaref [...] 2053-1.htm (lien trouvé dans Le topic-passage-obligé pour débutants)    

Est-ce toujours utilie de lever une exception ? Dans mes catch j'affichais d'où venait l'erreur et ça ne me semblait pas "mal".
Le problème serait que je le laisse continuer son execution comme si de rien n'était avec juste quelque messages informatifs.
Donc dans mon queryBroker je devrai lever les exceptions puisqu'elles sont graves.
 

Je suis désolée, je ne me prenais pas pour une débutante. Je croyais que ma gestion des exceptions n'était pas mauvaise mais je suis ouverte à toute critique
J'ai donc été voir le topic en question, mais j'ai pas l'impression d'y avoir apprit grand chose en fait
Peut être que mon défaut se limite à l'oublie de lever l'exception quand celle-ci le nécessite non ?

On ne peut pas être catégorique sur ce point. Ce qui compte, c'est qu'une condition d'erreur ne passe pas inapperçue et que tu ne perde pas sa trace. Parfois, le cas d'"exception" est fonctionnelement normal. P.e. tu veux tester si une variable String contient où un nom une valeur numérique : l'éventuel NumberFormatException ne doit pas forcément être renvoyé à la figure de l'appelant, c'est peut-être précisemment ce qu'on voulait.
 

Là, par contre, pas de doute.  
 

Beh voilà ! Au moins, tu progresseras, à l'inverse des nombreux boulets sur ce forum qui n'essayent pas de comprendre et qui veulent juste que ça marche.    
 

Tu manques simplement d'expérience et de pratique. Essaye de voir chaque méthode comme remplissant un contrat : p.e. "Obtenir la liste des clients". S'il n'y a pas de clients, tu renvoies une liste vide, c'est naturel. Ce n'est pas une erreur technique. Par contre, si en tentant d'obtenir cette liste, tu manges un SQLException, il y a un soucis. La méthode est-elle toujours capable de remplir son contrat ? Non. Peut-elle corriger l'erreur ? Non, alors, tu la rebalances. Finalement, c'est ton GUI qui va s'en occuper, en affichant un message d'erreur.
 
Une exception n'a rien de sale; n'hésite pas à en faire usage.
 
Mais rien ne remplacera l'expérience.

bon j'avais commencé par rajouter des throw mais du coup je suis obligée de rajouter des throws et des try catch partout c immonde.
est ce que je peux juste faire un printStackTrace sans lever l'exception ?

   
 
Les exceptions, ce n'est pas sale. Les clause throws, ce n'est pas sale.
 
Faire un print du stack trace au lieu de relancer l'exception, c'est mal. Très mal.
 
Et tu n'es pas obligée de faire un catch si ta méthode contient le mot-clef 'throws' (à cd que l'appelant ne se contente pas d'enterrer l'exception, mais ça, c'est son problème).

un gros +1 sur tout ce qu'a dit sircam
 
c'est justement parce que "c'est chiant" de devoir mettre des try/catch partout que les exceptions sont souvent mal gérés par les programmeurs java manquant d'expérience. C'est notamment un truc qui n'est pas du tout enseigné à la fac ou dans les école et c'est bien dommage.
 
Parce que tu verras que en fait, y a pas besoin d'en mettre tant que ca des try/catch. Souvent tu peux te contenter de mettre un throws de façon à ce que ce soit l'appelant qui traite l'exception.
 
Ton problème est l'exemple type d'une mauvaise gestion de l'exception : la méthode ne remplit pas son contrat, mais rien ne l'indique. Du coup tu te retrouves dans un état imprévu : ton Resultset est à null.
 
Au début, faut un peu se forcer à utiliser correctement les exceptions, mais tu verras que c'est payant. Au final ton programme est beaucoup plus robuste, plus facile à lire et à maintenir.

comment j'me fais tapper sur les doigts !
bon je m'en doutais un peu j'avoue
 
Alors j'ai une autre question : dans quels cas il est mieux de  mettre un throws plutot qu'un throw et vice versa ?
d'habitude je mettais des throws et le compilateur se démerdait avec.
 
Désolée de vous embêter, j'essaye d'améliorer ma façon de faire.

c'est pas méchant, promis
 

je comprends pas la question. Tu voulais pas dire "un throws plutot qu'un try/catch" ?
 
throws ca sert à indiquer que la méthode peut générer des exceptions
throw ca sert à lancer une exception

le throws m'embête
j'ai voulu faire ça :

il me semble que comme ça c'est ça va dans le sens que vous m'indiquez non ?
du coup je dois rajouter un throws SQLException dans ma méthode, bon OK spa grave.
Et du coup mon controller me pète une erreur. Alors j'me dis que j'vais aussi mettre le throws dans mon doPost ! Mais là il veut pas Il veut absolument que je mette un try catch.
 
mais dans mon doPost et mon doGet si je dois mettre des try catch partout ça va être immonde là quand même !

Ca m'étonnerait que le fait de mettre "throws" laisse le compilateur se débrouiller.
 
Comme le dit benou sert à indiquer que la méthode génère une exception. Mais c'est à toi de la lever et traiter.
 
Exemple : si on crée une méthode qui utilise des méthodes de traitement sur les flux sur fichier. Ces méthodes génèrent une IOException. Tu as donc deux choix :
 
1) Lever les IOException dans ta méthode même. Dans ce cas, pas besoin de "throws".
2) Utiliser "throws" pour dire que c'est la méthode qui appellera ta méthode qui devra lever l'exception.
 
Donc si la méthode que t'as crée s'appelle getData(), tu la déclares, dans le cas 2, comme suit :

Et si ton main() fait appel à cette méthode tu fais :

 
Donc je ne vois pas trop le rapport avec throws et throw    

Un bon coup de pelle à clous sur les doigts, ça fait parfois du bien
 

Les deux ne sont pas incompatibles ! Tu rédiges ta méthode. Si besoin est, à l'intérieur, tu as un block catch. Ce block peut relancer l'exception (throw e; ), soit en relancer une autre (throw new MyCustomException(e); ) ou encore régler le problème et ne rien relancer.
 
En fin de compte, s'il se peut que ta méthode génère une chekced exception, il te faudra la clause throws correspondante. C'est le cas si tu as un throw à l'intérieur de ta méthode, ou qu'une des méthodes appelées déclare elle-même 'throws'.
 
Concentre-toi sur le contrat que doit remplir ta méthode. Ensuite seulement, ce qui reste comme exceptions non-traitées (ou traitées mais relancées) sera déclaré avec le 'throws' adhoc.

1) on ne catch généralement pas NullPointerException : ce genre d'exception c'est généralement un bug, pas un comportement attendu.
 
2) As-tu vraiment besoin de loguer les SQLExceptions à cet endroit du code ?
 
3) (un peu lié au 2) déclarer un throws ca signifit "une erreur s'est produite. Je ne peux pas la contourner et remplir mon contrat. Je préviens donc l'appelant que mon contrat n'a pas été rempli".
Ca devient donc de la responsabilité de l'appelant de gérer cette erreur. L'appelant peut lui aussi décider de laisser passer l'exception ou de la traiter (avec un traitement approprié). Ca dépend si il est encore en état de remplir son propre contrat. et ainsi de suite ...
 
Dans ton cas c'est une servlet (j'imagine puisque tu parles de doPost). Le contrat de la méthode doPost c'est de répondre à une requête HTTP. Donc si le traitement que tu voulais faire en base a échoué et que tu t'es pris une SQLException, cette erreur doit effectivement remonter jusqu'à la Servlet. C'est à elle de traiter cette exception.
 
Ta servlet peut par exemple faire un response.sendError(500, "Erreur BDD" ). Ce serait aussi une bonne idée de loguer l'exception pour pouvoir éventuellement débugguer (si la cause de l'erreur est un bug). Pour moi, c'est plutot au niveau de la servlet que tu dois le faire plutot que dand la méthode getAllInstances.
 
4) Attention, quand tu veux loguer une exception, le mieux c'est de disposer aussi de sa stacktrace. Tu peux l'afficher comme ca :  e.printStackTrace(System.out);

merci pour ces explications j'commencais à m'embrouiller grave

C'est un plaisir d'aider les personnes qui font un effort pour comprendre. Je ne peux que redire mon dédain pour les boulets qui quittent ce forum avec un "ok ça marche merci" alors que la solution utilisée est en fait merdique et qui reveinnent 2 jours plus tard en ayant toujours pas essayé de comprendre.
 
 

bah merci à vous de prendre du temps pour m'aider c'est vraiment sympa
 
bon alors pour commencer je vais reprendre mon queryBroker et rajouter des throw pour lancer les exceptions adéquates.
ensuite reprendre mes classes métiers et virer les catch de NPE, puisque c pas terrible si j'ai bien compris. Mais à la place je vais devoir intercepter les exceptions lancées pour les traiter. c'est bien ça ? j'ai oublié quelque chose ?
 
Ca va m'obliger à changer pleins de trucs et j'ai peur d'être submergée et perdue du coup
surtout que je vois toujours pas pourquoi il me pète des NPE au final... puisque des fois tout va bien et des fois il la renvoie.

comme je le disais, je pense que tes classes métier doivent elles aussi laisser passer les exceptions : c'est à ta servlet des les traiter. Sinon, comment ta servlet pourrait savoir qu'il y a eu un problème dans le traitement métier ?
 
Il faudrait qu'on connaissent mieux ton projet pour pouvoir en dire plus.

je pensais plus aux exceptions lancées par le queryBroker (NamingException et SQLException) qui devraient être gérées par les classes métiers. Après tout ce sont elles qui accèdent à la BDD grace au QueryBroker.
 
exemples :
 

• QueryBroker

• Une méthode de ma classe métier Option qui accède au QueryBroker :

 
Ca ne va pas ça ?

dans le premier cas, moi je ne m'embeterais pas à loguer chaque cas d'erreur pour la NamingException. Je pense que je throwseerais simplement la NamingException. Charge à l'appelant de loguer l'erreur :  

 
remarque : évite de déclarer tes variables en les initialisant systématiquement à null. Je pense que tu fais ca de façon à ce que le compilateur ne te crache plus des message d'erreur du style "la variable peut ne pas avoir été initialisé", mais tu verras que si tu gère correctement tes exceptions, tu n'auras plus ces messages. Au contraire, ca te préviendra de bugs éventuels.
 
 
 
dans le 2e cas, le problème c'est que si y a une exception dans la méthode update(), l'appelant de cette méthode ne peut pas le savoir. Fait plutot ca :  

 
et mets le block try/catch dans la servlet qui apelle update. Il faudra d'ailleur que tu décide comment se comporte ta servlet en cas d'erreur de mise à jour de ta base.
 
 
remarque : ca ne me parait pas normal que tu puisses encore avoir des NamingException au moment où tu demandes une mise à jour de la base. Je pense que c'est à cause du QueryBroker.getInstance(). Est ce que tu ne pourrait pas récupérer l'objet QueryBroker une fois pour toute à la création de l'objet et stocker l'instance en variable de classe ?
 

le chemin menant à la perfection est long et sinueux

oui je vois ça mais c'est le résultat qui compte et je veux un boooo résultat
 
donc je suis d'accord avec toi, j'avais réfléchi et je crois bien que c'est à la servlet de gérer les exceptions.
 
pour la variable globale chui d'accord ça sera plus simple.
 
edit : petit pb avec le queryBroker en variable globale. je fais comment pour les méthodes static ? j'initialise également un queryBroker dans mes servlets que je passe en argument à mes méthodes statiques ?
 
edit 2 : je viens de dire une grosse connerie là oups faite comme si vous n'aviez rien lu !
 
edit 3 : par contre il me pète une erreur "unhandled Naming exception" sur ma variable globale (définit comme ceci : private final static QueryBroker _QUERY_BROKER = QueryBroker.getInstance()
heu... ouais c bien beau mais là heu...

+1 :
 

 
Ou encore la 2è proposition de Benou.
 
Ce que je n'aime pas, c'est qu' "en haut", tu te retrouves à catcher "Exception" parce les différentes méthodes déclarent lancer elles-mêmes un paquet d'exception... Tu te retrouves rapidement à les relancer avec

Et pour simplifier, tu ne te foules plus :

Mais tu perds des infos. Dès que le système devient sérieux, mieux vaut se créer sa propre classe d'exception "intelligente" :

Cette classe comprenant une nested exception, un identifiant, etc. Mais ça, c'est déjà un peu plus avancé, ne t'en fais pas.

ha oui mais non ... moi je parlais de variable de classe, pas de variable static ...
 
genre :
 

ah daccord
 
par contre je suis obligée de la mettre en static parce que certaines de mes méthodes le sont et donc j'ai pas le choix, m'enfin c pas trop grave je pense.
 
par contre j'ai pas d'init dans ma servlet parce que je n'initialise pas mes classes métiers comme ça mais selon les paramètres récupérés par mes doGet et doPost.

sureprenant d'avoir des méthodes statiques sur une classe récupérée via le design pattern singleton ...
 

ca veut dire que tu les réinitialises à chaque requête ... c'est pas ce qu'il y a de plus optimal