Discussion :

[mithrendil]

Bonjour, j'ai un petit projet pour lequel je dois vérifier la présence d'au moins un fichier portant l'extension ".map" dans un répertoire donné par l'utilisateur. Voici mon code:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
private void button3_Click(object sender, EventArgs e)
        {
            string[] files;
            bool map_exists = false;
            folderBrowserDialog1.ShowDialog();
            files = Directory.GetFileSystemEntries(folderBrowserDialog1.SelectedPath);
            int filecount = files.GetUpperBound(0) + 1;
            for (int i=0; i<filecount; i++)
            {
                File temp = new File (files[i]);
                if (temp.Extension.ToString("map"))
                {
                    map_exists = true;
                }
            }
            if (!map_exists)
            {
                textBox3.Text = "Aucune map dans ce répertoire!";
            }
            else{
                textBox3.Text = folderBrowserDialog1.SelectedPath;
            }
        }

La compilation me renvoit l'erreur suivante:

Erreur 1 Impossible de déclarer une variable de type static 'System.IO.File' D:\Documents and Settings\Admin\Local Settings\Application Data\Temporary Projects\mbuilder\Form1.cs 64 17 mbuilder

Pourquoi ne puis je pas instancier un objet "File"? Merci de votre aide.

[ced600]

File est une classe static, comme te le dit ton erreur, et donc par définition tu ne peux pas l'instancier.

Pour créer un fichier tu peux utiliser la méthode Create ou CreateText qui te retourne un StreamWriter, buffer qui encapsule ton fichier nouvellement créé, et que tu dois utiliser pour écrire des données dans le fichier.

Si tu veux créer un fichier et obtenir des informations du fichier comme l'extension, tu doit créer un Objet FileInfo. Alors à partir de celui-ci tu pourra créer le fichier, et obtenir des informations sur son extension, ....

Et sinon, je te conseille d'utiliser des Foreach lorsque tu parcours entièrement un tableau, ou une collection, ...., c'est plus simple et tout aussi efficace.

[smyley]

moi je dirai :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
 
public virtual bool MapExists(string directory)
{
 DirectoryInfo info = new DirectoryInfo(directory);
 if(!info.Exists)
   return false;
 else
  return info.GetFiles("*.map").Length > 0; //Length ou Count, je fait de tête ^_^
}

[mithrendil]

Merci pour vos réponses, j'ai résolu le problème en utilisant un objet fileinfo. Mais j'ai un peu peur que le tableu ainsi que la boucle chargent un peu trop le programme en mémoire.

[Maniak]

smyley n'est pas loin, mais il y a encore plus bateau :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
string path = folderBrowserDialog1.SelectedPath;
bool exists = Directory.GetFiles( path, "*.map" ).Length > 0;

C'est quand même un poil plus simple/clair/court non ? :)

[mithrendil]

C'est vrai c'est bien plus léger comme ça. Merci!

[smyley]

sC'est quand même un poil plus simple/clair/court non ?

Hihi, je préfère toujours vérifier l'existence des choses que je touche, tic que j'ai prit en ayant marre d'avoir des crash car un élément n'existait pas

[ced600]

J'opterais plus pour cela :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
public virtual bool MapExists(string directory)
{
try
{
	bool existe = false;
	if (Directory.Exists(path))
		existe = Directory.GetFiles(path, ".*map").Length > 0;
	return existe;
}
catch(DirectoryNotFoundException)
{
	//Traitement de l'erreur
}
catch(Exception)
{
	//Traitement de l'erreur
}
finally
{
	return false;
}
}

[smyley]

ça ne sert à rien de catcherDirectoryNotFoundException car tu as déjà vérifié que le dossier existe

[ced600]

Deux sécurité vallent mieux qu'une si tu veux un programme robuste.

[bilb0t]

Mais mettre des catch qui ne servent à rien, je vois pas l'interret...

[ced600]

C'est juste une question de prévenir et de guérir.
Plutot que choisir l'un des deux, je choisis les deux. Comme cela si pour une raison bizzare que je ne comprends pas, qui peut être du à la complexité de mon programme, le test de vérification de l'existence du répertoire n'est pas faite, je peut quand même m'en sortir grâce au try catch finally, si ce repertoire n'existe pas.
Il ne faut pas voir mon bout de code en tant que seul code de l'application.
on peut imaginer un code écrit comme ceci un soir et qui pose des problème de débogage le lendemain:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
public virtual bool MapExists(string directory)
{
try
{
	bool existe = false;
	if (AutreFonction())
		existe = Directory.GetFiles(path, ".*map").Length > 0;
	return existe;
}
catch(DirectoryNotFoundException)
{
	//Traitement de l'erreur
}
catch(Exception)
{
	//Traitement de l'erreur
}
finally
{
	return false;
}
}
private bool AutreFonction()
{
  bool pathexiste = true;
  try
  {
    EncoreUneAutreFonctionAppele();
    if(!Directory.Exists(path))
      pathexiste = false;
    return pathexiste;
   }
   catch(Exception)
   {
   }
   finally
   {
     return pathexiste;
   }
}

Imaginons que "EncoreUneAutreFonctionAppele" génère une exception toutes les 1000 exécution du programme, alors sans try catch dans la fonction MapExists tu reçois une erreur et cela peut faire planter ton appli au mieux, et au pire faire n'importe koi.
Avec try catch finally, tu peut arriver à te sortir du problème et corriger le problème par exemple en rappelant la fonction qui passera peut être correctement au deuxième essaie, et au pire des cas, tu gères l'erreur et evite que ton appli fasse n'importe koi.

[smyley]

Il faut quand même savoir ce que tu fait dans ton code :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
bool existe = false;
	if (AutreFonction())
		existe = Directory.GetFiles(path, ".*map").Length > 0;
	return existe;

Ce n'est pas sur, et de plus, les try-catch sont là quand une situtation n'a pas été prévue normallement car c'est un procéssus très couteux en performances que de lancer une exception, en réalité le code cis-dessus n'est pas pas "correcte" :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
        bool existe = false;
	if (Directory.Exists(path) && AutreFonction())
		existe = Directory.GetFiles(path, ".*map").Length > 0;
	return existe;

Dans ce cas on est quasiment sur que le dossier existe et pour attraper les érreurs qui pouraient venir de l'utilisation de Directory, il faut catcher IOException et pour les erreur venant de AutreFonction il faudrait savoir quelles exceptions pourraient être lancée ou alors attraper uniquement Exception, qui regroupe le tout.
De plus si dans ton AutreFonction() tu catch déjà Exception, aucune érreur ( à par peut être les erreurs du CLR genre "Plus de mémoire" ) ne peuvent arriver dans ta première fonction donc tu as un catch qui ne servira jamais ...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
public virtual bool MapExists(string directory) 
{ 
     bool existe = false; 
     try {  
      if (Directory.Exists(path) && AutreFonction())  
          existe = Directory.GetFiles(path, ".*map").Length > 0; 
       return existe; 
       } catch(IOException) 
        {  //Traitement de l'erreur } 
      return existe;
        }
} 
 
private bool AutreFonction() 
{   
 try  {  EncoreUneAutreFonctionAppele(); 
      return Directory.Exists(un_path);
 }  catch(Exception) 
 {
   return false;
 }
}

[ced600]

Je sais que le code que j'ai ecrit n'est pas le meilleur code que l'on peut ecrire et que cela peut provoquer des problèmes, puisque je l'ai fait exprès.
Je voulais mettre en évidence certains problèmes qui peuvent se produire et prouver que avoir deux sécurités c'est meilleur.
Bon imaginons que "AutreFonction" je n'y ai pas accés et soit développer par quelqu'un d'autre. Que la spec de cette fonction me précise qu'elle vérifie l'existence du répertoire et elle renvois vrais ou faux en fonction de son existence (et elle fait aussi d'autre truc). Il est aussi précisé qu'elle ne fait que remonter des IOException et qu'elle appelle une autre fonction dont je ne sais rien.
Mais je suis obligé de l'appeler. Elle ne plante qu'une fois sur 1000 lancement, mais ça personne ne le sait. Dans ce cas je peut te dire qu'il vaut mieux l'entourer de try catch. C'est moins risquer pour l'application. En plus tu peut dév ta propre classe d'exception et écrire dans un fichier log les erreurs et plus facilement trouver le problème.

En faire tomber une application maitresse qui lance et gère 10 thread, dont 3 ne doivent jamais s'arreter, à cause d'un seul thread non indispensable qui plante, c'est un peu con . Il y a des cas où prendre une deuxième sécurité peut être indispensable.

C'est tout ce que je voulais dire.

[bilb0t]

Je maintiens qu'une bonne analyse, une bonne politique de rédaction de code t'évite se genre de problème...

Le gros try catch autour du code "parce qu'on est pas trop sur et que 2 précautions valent mieux qu'une et que..." prouve surtout une non compétence qu'une sécurité à mon sens.

Maintenant si tu travailles avec des méthodes dont tu n'es pas trop sur, c'est peut-être autour du developpeur de ces méthodes qu'ils faut mettre un gros try catch...

[Maniak]

Hihi, je préfère toujours vérifier l'existence des choses que je touche, tic que j'ai prit en ayant marre d'avoir des crash car un élément n'existait pas :king:

Là ça dépend de ce que doit faire le programme :)
Si le cas où le répertoire examiné n'existe pas est censé être anormal, il vaut mieux laisser passer DirectoryNotFoundException, sinon ça masque l'erreur :)

(enfin dans tous les cas, vaut mieux ça que coller des try/catch partout pour essayer de gérer toutes les erreurs possibles et imaginables qui pourraient arriver en interne dans toutes les méthodes qu'on appelle, surtout quand on ne sait pas quoi faire avec les exceptions qu'on récupère)

(non, je ne vise personne :)

(et si on veut un programme robuste, deux sécurités ne valent pas mieux qu'une si elles sont redondantes, et aucun nombre de sécurités ne vaut des tests unitaires automatisés)

(je dis ça, je dis rien)

[ced600]

Je suis d'accord pour les tests. Je ne comprends pas cette esprit anti try catch. La redondance des sécurités est quelque chose d'indispensable pour une application robuste.
Il existe des cas où vous ne pouvez pas vous permettre que l'application plante.

[Maniak]

Je ne comprends pas cette esprit anti try catch.

Pas anti-try/catch, loin de là. Anti-redondance. La redondance c'est de la duplication. La duplication dans le code, c'est le Mal.

La redondance des sécurités est quelque chose d'indispensable pour une application robuste.
Il existe des cas où vous ne pouvez pas vous permettre que l'application plante.

Vi, donc il faut couvrir les zones à plantage pour pouvoir retomber sur nos pattes. Il y a besoin d'une bonne couverture, pas de redondance.
Le code, c'est pas une série d'alims. Y en a pas une qui risque de tomber en panne et qui oblige à en avoir d'autres en réserve.

Si les risques de plantage d'un cheminement dans le code sont couverts une fois, c'est bon, ils sont couverts. S'ils peuvent parfois laisser passer des erreurs, c'est que le code est mal fait (du moins qu'il y a eu des oublis, ça arrive :) et qu'il faut l'améliorer. Et améliorer ne veut pas dire rajouter 3 couches d'autres contrôles et autres try/catch en pagaille pour être SÛR-SÛR-SÛR que rien ne passera. Ça c'est pas une amélioration. C'est une noyade de poisson. D'autre part, un bout de code n'a à gérer que les risques de plantage de ce dont il est responsable. Pas tous les plantages possibles et imaginables de tous les objets qu'il utilise (sauf cas de librairies externes mal foutues, et pour ça il suffit de faire un wrapper qui s'occupe de gérer les appels à risques et de fournir en sortie un comportement fiable).

Comment on peut garder un code clair, simple et lisible s'il est badigeonné de tests et contrôles sur les mêmes choses ? C'est une marque de manque de confiance dans son code. Et ça, ça s'améliore. Idéalement en pratiquant les tests unitaires (idéalement en TDD), mais même sans, ça se travaille.


Il faut aussi faire la distinction entre "gérer les erreurs" et "empêcher que l'appli plante". Si l'impératif est que l'appli ne plante pas, forcément il faut brasser plus large, à un niveau plus élevé, pour que les erreurs dont on n'est pas responsable soit quand même interceptées, et au moins loggées, à défaut de pouvoir en faire autre chose. Mais tout ça, c'est à un niveau quand même un poil plus élevé que la moindre méthode paumée au fin fond d'une librairie interne.

Tout ça pour dire que quand on a testé qu'un répertoire existe et qu'on a reçu en réponse qu'il existe, c'est bon, il existe. S'il y a quelque chose à blinder, c'est la méthode qui teste l'existence du répertoire. Pas *tout* le code autour de *tous* les appels à cette méthode...

[ced600]

Bon je ne dirais plus rien, sauf que je ne manque pas de confiance dans mon code, enfin pensez ce que vous voulez.

De toute façon il est inutile que j'en dise plus, chacun peut se forger son propre avis, et nous sommes en désacord total.

La vie nous dira qui a raison de toute façon. (En général elle est malicieuse et nous aurons des expérience qui nous conforterons dans nos "dogmes" )

Et puis faut que j'arrête de poster sur ce sujet parce que comme je suis tétu et que je cherche toujours à avoir le dernier mot, j'ai pas fini d'en écrire des messages .

En tout cas une chose essentiel fut dites lors de cette discussion, faites des tests unitaires, d'intégration, de validation, et de non régression !!!
Dommage que beaucoup de d'entreprise ne pense pas de cette façon en informatique.