Discussion :

[mactwist69]

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 :

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
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 :

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

[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 .

[Keihilin]

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

-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...

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

Réponse 2

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 :

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
 
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().

[mactwist69]

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.

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?

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?

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...

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()
??

[mactwist69]

J ai donc tester ton code...

Il me mets un warning sur:

Code :

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

  dr_sql.Close()

et me dis:

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.

[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.

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.

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.

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).

[mactwist69]

J ai donc tester ton code...

Il me mets un warning sur:

Code :

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

dr_sql.Close()

et me dis:

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?

[mactwist69]

Je recapitule mon seul et unique probleme.

avec le code suivant ca "marche":

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
 
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 :

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
 
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 :

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
 
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???

[Keihilin]

Le waring est normal et non-bloquant.

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

Code :

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

1
2
Dim dr_sql As new SqlClient.SqlDataReader

tu peux la laisser avant le try.

[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.

[banjo12]

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

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
 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