Close or not close

[thierryler]

J'ouvre un post spécifique à la discussion commencée ici http://www.developpez.net/forums/d11...a/#post6251531 et relative au double close.

Code :

1
2
3
4
5
6
7
 
 
FileReader fr = new FileReader(f);
BufferedReader br = new BufferedReader(fr);
 
br.close()
fr.close();

Le point porte sur le fait de faire ou non le "close()" du FileReader.

La doc dit grosso modo que ce n'est pas nécessaire.

Par contre, d'expérience (douloureuse), je dis que ça ne mange pas de pain de tout fermer.

De manière générale, ma stratégie est de fermer moi-même ce que j'ai ouvert. Quand ce n'est pas moi qui ouvre alors je laisse la lib se démerder.

Avant je faisais bien confiance à la doc. Maintenant je marche autrement. Si le code vient de mon équipe, je ne blinde pas (quoi que). Si le code vient d'un étranger alors j'utilise systématiquement des techniques de défense (copie de défense, assert, etc.) et je diminue progressivement le niveau de défense si tout se passe bien.

[tchize_]

La javadoc de Reader dit que close() ferme le stream, comme ton bufferedreader et ton filereader partagent le même stream, un seul devrait suffire.

Mais:
la javadoc n'est pas explicite sur ce point
comme tu le dit, ça ne mange pas de pain
C'est plus propre de systématiquement le mettre. Le jour où tu trouve un code où il y a moins de close que de new tu vois tout de suite qu'il manque peut-être quelquechose

Maintenant, java 7 avec ses try with ressource devrais te faciliter le travail

[thierryler]

Maintenant, java 7 avec ses try with ressource devrais te faciliter le travail

Je ne sais pas trop. Je me dis que là où c'était foireux en java 6, ça sera encore plus foireux en Java 7 car on ne le verra même plus... Et puis Java 7 ne fera ça que sur les Closables.

[_skip]

Envoyé par tchize_

la javadoc n'est pas explicite sur ce point

Il y a un article technique de SUN sur les flux en java qui dit clairement que seul le stream le plus haut a besoin d'être fermé.
http://java.sun.com/developer/techni...ProgIOStreams/
En connaissant le pattern utilisé (decorator), on peut être raisonnablement sûr.

Envoyé par tchize_

comme tu le dit, ça ne mange pas de pain
C'est plus propre de systématiquement le mettre. Le jour où tu trouve un code où il y a moins de close que de new tu vois tout de suite qu'il manque peut-être quelquechose

Question de philosophie, pour ma part je n'aime pas trop le code qui sert à rien.

Je ne sais pas trop. Je me dis que là où c'était foireux en java 6, ça sera encore plus foireux en Java 7 car on ne le verra même plus...

Perso, c'est l'un des changements les plus intéressants de java7 à mon sens.

Dans un bon nombre d'API C++, la libération des ressources extérieures se fait dans le destructeur de façon transparente. En java on te demande de le faire manuellement car l'appel à finalize, qui contient une sécurité, est imprévisible.

Cependant chez java en *bonus*, tu dois gérer le cas où la ressource est nulle car l'initialisation a échoué, et le cas où close() lance une exception, cela fait 10 bonnes lignes de plomberies finalement juste pour une opération I/O de base.
Cette nouvelle syntaxe fait exactement ce que tu ferais en plus concis , indique dès la première ligne ton intention d'en finir proprement avec la ressource et te donne la garantie que tout le nettoyage qui pouvait être fait a été fait.

C'est clair, même sans cette syntaxe ça peut se factoriser jusqu'à un certain point.
(voir par exemple ici) http://commons.apache.org/io/api-1.2...InputStream%29

Mais c'est tout de même nice to have.

[thierryler]

Ouais, quand la lib vient d'Oracle-Sun, j'ai globalement confiance. Mais quand elle vient d'un éditeur tiers, je ne sais pas sur quel pied danser. L'exemple de code que j'avais donné le prouve. La javadoc n'engage que celui qui la lit, comme en politique avec les promesses.

Pour le try (resources) de java 7, c'est clair que c'est top, à condition de savoir ce qu'on fait. Ce qui me fait peur, c'est que la lib tiers ne respecte pas son contrat et que, du coup, comme java 7 cachera tout ça, on risque de faire des mega bourdes sans même s'en rendre compte

[_skip]

Envoyé par thierryler

Ouais, quand la lib vient d'Oracle-Sun, j'ai globalement confiance. Mais quand elle vient d'un éditeur tiers, je ne sais pas sur quel pied danser. L'exemple de code que j'avais donné le prouve. La javadoc n'engage que celui qui la lit, comme en politique avec les promesses.

Pour le try (resources) de java 7, c'est clair que c'est top, à condition de savoir ce qu'on fait. Ce qui me fait peur, c'est que la lib tiers ne respecte pas son contrat et que, du coup, comme java 7 cachera tout ça, on risque de faire des mega bourdes sans même s'en rendre compte

Oui, tu as raison de dire qu'il faut se méfier quand ça provient d'éditeurs tiers, mais c'est quelque chose que l'on doit (devrait tout au moins) faire en règle générale. Plus facile à dire qu'à faire quand on sait que java c'est aussi le monde des gros frameworks que la majorité utilise sans forcément en connaître les détails.
Voodoo magic or not , il faut choisir à qui faire confiance, perso je suis friand des libs apache qui sont généralement de bonne qualité. Tout comme je n'hésite pas trop à "dropper" certaines libs lorsque la documentation est peu claire ou que le projet sent le souffre.

[thierryler]

Cela dit, vu que tu parles d'apache, faudrait demander aux gars de Lucene leur avis sur java 7 lol

[tchize_]

CA ma rappelle un bug report chez atlasian concernant leur outil jira tiens. Ils voulaient pas me croire quand je leur disais que leur truc faisait un System.out.close() ce qui m'empechait de voir les messages des autres application sur le même tomcat.

J'ai du monter listener tomcat qui remplacait system.out par un truc custom qui se contentait d'afficher un stacktrace quand on tentait le close. Je leur ai envoyé le stacktrace

[_skip]

Envoyé par tchize_

CA ma rappelle un bug report chez atlasian concernant leur outil jira tiens. Ils voulaient pas me croire quand je leur disais que leur truc faisait un System.out.close() ce qui m'empechait de voir les messages des autres application sur le même tomcat.

J'ai du monter listener tomcat qui remplacait system.out par un truc custom qui se contentait d'afficher un stacktrace quand on tentait le close. Je leur ai envoyé le stacktrace

Ils ont sans doute instancié un writer par dessus System.out et, à un moment donné, tenté un writer.close() qui conformément à l'API, a appelé System.out.close().

[tchize_]

nan ils utilsiaient une librairie cvs buggée qui utilisait dans certains cas system.out pour afficher les messages cvs, mais fermait le stream à la fermeture de connexion, indémendament de si il s'agissait de system.out ou un stream custom