Try, catch dans une fonction

Version imprimable

Bonjour bonjour!

Voila, je developpe un logiciel depuis un ptit moment, comme c est bientot l heure de le mettre en service, je me suis mis a le rendre propre et a mettre du try catch un peu de partout.

J ai donc etudier, lu etc etc.

Vioci un exemple:
Code:

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 Public Function Site_name_with_ItemId(ByVal id As Integer) As String 'Return a site name with a Item ID Try Form_Main.SqlConnection1.Open() Dim com_sql As New SqlClient.SqlCommand Dim dr_sql As SqlClient.SqlDataReader com_sql.Connection = Form_Main.SqlConnection1 com_sql.CommandType = CommandType.Text com_sql.CommandText = "SELECT Name_site FROM Site WHERE ID_site=(SELECT IDFK_site_item FROM Item WHERE ID_item='" & id & "')" dr_sql = com_sql.ExecuteReader Dim ok As String If dr_sql.Read() = True Then If dr_sql.IsDBNull(0) Then ok = "" Else ok = dr_sql.GetString(0) End If Else ok = "" End If dr_sql.Close() Form_Main.SqlConnection1.Close() Return ok Catch ex As Exception MsgBox(ex.Message) add_log(ex.Message, "Site_name_with_ItemId") End Try End Function
Une petit fonction qui va chercher une info dans la base de donnees... Tout marche tout va bien...

Mais VB2005, me souligne en gris End Function et me dis :

Citation:

Function Site_name_with_ItemId doesn't return a value on all paths. A null reference exception could occur at run time when the result is used

Les questions:

-Bien sur que peut etre la function ne retournera peut etre rien (j espere pas) mais c est bien pour ca que je mets un try catch non?
-J ai donc essayer de mettre un Finally avec un return quand meme, mais il aime pas.
- Est ce que je dois ignorer ce warning, ou fais je quelque chose de pas propre?

Merci

10/07/2008, 11h44
banjo12

Rajoute un return dans ton catch.
Je m'explique : si ton code leve une exception, il sautera dans le catch et executera le catch... Une fois les deux lignes exécutées, il termine la fonction sans rien renvoyer !!
Tu peux retourner un code d'erreur par exemple .

Citation:

Envoyé par mactwist69

je me suis mis a le rendre propre et a mettre du try catch un peu de partout.

Attention. La "propreté" du code n'a rien à voir avec la présence de try...catch un peu partout :roll:

Citation:

Envoyé par mactwist69

-Bien sur que peut etre la function ne retournera peut etre rien (j espere pas) mais c est bien pour ca que je mets un try catch non?

non...

Citation:

Envoyé par mactwist69

- Est ce que je dois ignorer ce warning, ou fais je quelque chose de pas propre?

Réponse 2 :mrgreen:

Avant de vouloir placer des try..catch, il faut refléchir à l'utilité et aux opérations qui peuvent échouer.

Par ailleurs, il faut aussi refléchir au comportement de ta fonction avant de songer à renvoyer une chaîne vide...Une exception est plus parlante pour le code appelant dans ce cas.

Bref, je pense que ceci est un poil plus propre :
Code:

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 Public Function Site_name_with_ItemId(ByVal id As Integer) As String Dim com_sql As New SqlClient.SqlCommand Dim dr_sql As SqlClient.SqlDataReader Dim ok As String com_sql.Connection = Form_Main.SqlConnection1 com_sql.CommandType = CommandType.Text com_sql.CommandText = "SELECT Name_site FROM Site WHERE ID_site=(SELECT IDFK_site_item FROM Item WHERE ID_item='" & id & "')" Try Form_Main.SqlConnection1.Open() dr_sql = com_sql.ExecuteReader If dr_sql.Read() = True Then If dr_sql.IsDBNull(0) Then ok = "" Else ok = dr_sql.GetString(0) End If Else ok = "" End If Return ok Catch ex As Exception add_log(ex.Message, "Site_name_with_ItemId") Throw Finally dr_sql.Close() Form_Main.SqlConnection1.Close() dr_sql.Dispose() End Try End Function
Mais il subsiste des erreurs globales de conception.

Je ne vois pas ce que fait ton objet Connection sur ta Form_Main.
Cette fonction est une fonction "métier", ce n'est pas à elle d'afficher un MessageBox en cas d'erreur (d'ailleurs je l'ai supprimer du catch).
Essais d'utiliser plutôt des requêtes paramétrées au lieu de la construire
à grand coup d'inclusion de variables.
J'ai laissé tel quel, mais il faudrait réduire le scope du catch. A priori si quelque chose foire ça sera l'ouverture de la connexion ou le ExecuteReader().

10/07/2008, 12h48
mactwist69

reponse

Merci bien a vous deux pour vos reponses

tout particulierement la deuxieme. J ai bien conscience de pas etre un programmeur hors pair, et ca me permet de revenir sur mes petits problemes.

Citation:

Je ne vois pas ce que fait ton objet Connection sur ta Form_Main.

Et bien, j ai ajouter ma connexion a ma forme main, et parametrer. Je me suis demander si c etait vraiment utile d en creer une par Form. Quel interet?

Citation:

Cette fonction est une fonction "métier", ce n'est pas à elle d'afficher un MessageBox en cas d'erreur (d'ailleurs je l'ai supprimer du catch).

Tu sous entends donc qu au retour de valeur, c est la procedure apelante qui enverra un message? Est ce juste un probleme de bien faire les choses?

Citation:

Essais d'utiliser plutôt des requêtes paramétrées au lieu de la construire
à grand coup d'inclusion de variables.

La je vois pas... Tu parles du fait d avoir une fonction avec un parametre, et que j'utilise ce parametre pour construire ma requete???
Je vois pas comment je pourrai faire autrement.... Mes objets sont dynamiques, meme si je voulai je ne pourrai pas creer toutes les requetes possibles...

Citation:

J'ai laissé tel quel, mais il faudrait réduire le scope du catch. A priori si quelque chose foire ça sera l'ouverture de la connexion ou le ExecuteReader().

-Reduire le scope ne va pas alterer l execution du code?
-Et justement, les bugs... Si on pouvais a priori les prevoirs, y en aurai plus non?
Enfin je comprends que le try n est pas forcemet utils sur des declaration de variable tout de meme.

Derniere petite question.... A quoi correspond ton Throw et dr_sql.Dispose()
??
10/07/2008, 13h08
mactwist69

test

J ai donc tester ton code...

Il me mets un warning sur:

Code:

dr_sql.Close()

et me dis:

Citation:

Variable dr_sql is used before it has been assigned a value. A null reference exception could result at runtime

Une idee?? Je sais j pose beaucoup de question. ;)
10/07/2008, 13h33
Keihilin
Avant de répondre en détails, juste une petite précision :

La principale utilité d'un bloc try/catch/finally ne réside pas (contrairement à l'idée répandue) dans le catch, mais dans le finally !
Il faut que tu comprennes que lorsqu'une erreur se produit et qu'une exception est levée, le flux d'exécution du programme est interrompu et donc potentiellement laissé dans un état "impropre".
Le finally te permet d'être sûr que si une erreur se produit dans ton bloc try, certaines instructions seront quand même exécutées; typiquement du nettoyage d'objets crées dans le try ou avant.

Par ailleurs, 99% des catch que je vois sont soit :
- Totalement inutiles, voir néfastes...
- Juste là pour permettre de loguer l'erreur et la renvoyer.
Citation:

Envoyé par mactwist69

Tu sous entends donc qu au retour de valeur, c est la procedure apelante qui enverra un message?

Exactement. Ta fonction "métier" n'a pas à se préoccuper de problèmes d'affichage. Soit elle s'exécute bien et renvoit une valeur attendue, soit elle ne s'exécute pas bien et renvoit une exception. C'est à la fonction appellante de la couche supérieur de décider quoi faire de cette exception.

Citation:

Envoyé par mactwist69

La je vois pas... Tu parles du fait d avoir une fonction avec un parametre, et que j'utilise ce parametre pour construire ma requete???

Non non, je parle de requêtes SQL paramétrées...
Fais une recherche dans les tutos, tu comprendras mieux.

Citation:

Envoyé par mactwist69

Derniere petite question.... A quoi correspond ton Throw et dr_sql.Dispose()
??

Le Throw renvoit l'exception précédemment capturée par le catch.
Dans ton cas, le catch n'est pas totalement inutile puisque tu log l'erreur, mais après ça, il ne faut pas oublier de faire remonter cette exception plus haut, sinon tu ne sauras pas que qqch s'est mal passé...

Quand au Dispose(), je t'invite à faire des recherches car c'est une notion très importante dans le monde .Net (cherche Dispose ou IDisposable).
10/07/2008, 13h42
mactwist69

J ai donc tester ton code...

Il me mets un warning sur:

Code:

dr_sql.Close()

et me dis:

Citation:

Variable dr_sql is used before it has been assigned a value. A null reference exception could result at runtime

Ca m embete de laisser ca comme ca en warning.

Ca doit etre par ce qu il y a un risque que le code n atteigne pas son atribution de valeur??

Y a donc un probleme de logique. Comment y pallier?? Se serais dommage de revenir en arriere non?

hum hum

Je recapitule mon seul et unique probleme.

avec le code suivant ca "marche":

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
Try
      Dim com_sql As New SqlClient.SqlCommand
      Dim dr_sql As SqlClient.SqlDataReader
      com_sql.Connection = Form_Main.SqlConnection1
      com_sql.CommandType = CommandType.Text
 
      com_sql.CommandText = "SELECT Name_site FROM Site WHERE ID_site=(SELECT IDFK_site_item FROM Item WHERE ID_item='" & id & "')"
 
       dr_sql = com_sql.ExecuteReader
 
       blablabla
 
      dr_sql.Close()
            Form_Main.SqlConnection1.Close()
 
 Catch ex As Exception
 
            add_log(ex.Message, "Site_name_with_ItemId")
             throw
 End Try

Comme ca il y a un warning sur dr_sql:

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
Dim com_sql As New SqlClient.SqlCommand
Dim dr_sql As SqlClient.SqlDataReader
com_sql.Connection = Form_Main.SqlConnection1
com_sql.CommandType = CommandType.Text
com_sql.CommandText = "SELECT Name_site FROM Site WHERE ID_site=(SELECT IDFK_site_item FROM Item WHERE ID_item='" & id & "')"
 Try
       dr_sql = com_sql.ExecuteReader
 
       blablabla
 
 Catch ex As Exception
 
       add_log(ex.Message, "Site_name_with_ItemId")
       throw
finally
           dr_sql.Close()
            Form_Main.SqlConnection1.Close()
End Try

Et comme ca pas du tout: Erreur toujours sur dr_sql:

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
Try
          Dim com_sql As New SqlClient.SqlCommand
          Dim dr_sql As SqlClient.SqlDataReader
            com_sql.Connection = Form_Main.SqlConnection1
            com_sql.CommandType = CommandType.Text
 
            com_sql.CommandText = "SELECT Name_site FROM Site WHERE ID_site=(SELECT IDFK_site_item FROM Item WHERE ID_item='" & id & "')"
 
            dr_sql = com_sql.ExecuteReader
 
             blablabla
 
 Catch ex As Exception
 
            add_log(ex.Message, "Site_name_with_ItemId")
            throw
finally
         dr_sql.Close()
         Form_Main.SqlConnection1.Close()
End Try

Le probleme viens donc de :

- si dr_sql est declare avant le try, c est un warning, sur dr_sql.close (genre la variable n a peut etre pas de valeur)

- si dr_sql est declare apresle try, c est une erreur, sur dr_sql.close (genre la variable n a peut etre pas ete declarer)

La solution qui "marche" fais un dr_sql.close() dans le try....

Etant debutant et suivre des conseils, j aimerai quand meme effectuer ce try avec ce finally pour close() tout ce qu il faut au cas ou....

Quelqu un aurai t il une idee???

10/07/2008, 15h45
Keihilin
Le waring est normal et non-bloquant.

Mais tu peux le faire disparaître en remplaçant la déclaration :
Code:

1 2 Dim dr_sql As new SqlClient.SqlDataReader
tu peux la laisser avant le try.
10/07/2008, 15h48
mactwist69

merci!

Heu.... En fait, ca n a pas marcher, puisque SqlClient.SqlDataReader n a pas de constructor.

Donc le new ne fonctionne pas.

Par contre de declarer:

Public dr_sql as SqlClient.SqlDataReader

En en tete, le fais marcher.

Pour info, voici ce que propose VS 2005 par ces snippets codes :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 Dim reader As SqlDataReader
    Dim previousConnectionState As ConnectionState = conn.State
Try
If conn.State = ConnectionState.Closed Then
	conn.Open()
End If
reader = cmd.ExecuteReader()
Using reader
	While reader.Read
    ' Process SprocResults datareader here.
		Console.WriteLine(reader.GetValue(0))
	End While
End Using
Finally
    If previousConnectionState = ConnectionState.Closed Then
        conn.Close()
    End If
End Try

Et la pas de warning