critique du code source de mon programme

Version imprimable

Bonjour à tous,

Je m'arrête un peu dans mon petit projet pour vous soumettre une partie de mon code source.
Comme le dev n'est pas mon job, je souhaite que vous regardiez ce que j'ai pondu pour que vous me donniez des voies d'explorations sur ce que je pourrais amélioré, notamment aussi au niveau des commentaires.

Voici ce qui ce passe quand je clique

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
36
37
38
39
40
41
42
43
44
45
46
47
48
49
 
Private Sub Image2_Click()
Dim i, fin_client, deb_contrat, fin_contrat, deb_support, fin_support, nb_feuille As Integer
If chemin <> "" Then
 
'Ouverture de l'application excel
Set appExcel = CreateObject("Excel.Application")
 
Set wbExcel = appExcel.Workbooks.Open(chemin)
'wsExcel correspond à la première feuille du fichier
 
 
'si l'utilisateur souhaite importer l'en cours par client
If Check1.Value = 1 Then
    'Test pour savoir si les feuilles existe
    If feuille_existe("en cours client") = False Then
        Module1.ajout_feuille ("en cours client")
    End If
End If
 
'si l'utilisateur souhaite importer l'en cours client par contrat
If Check2.Value = 1 Then
    If feuille_existe("en cours client par contrat") = False Then
        Module1.ajout_feuille ("en cours client par contrat")
    End If
End If
 
'si l'utilisateur souhaite importer l'en cours client par support
If Check3.Value = 1 Then
    If feuille_existe("en cours client par support") = False Then
        Module1.ajout_feuille ("en cours client par support")
    End If
End If
 
 
'On  se positionne sur la dernière feuille (feuille tousclient)
nb_feuille = Sheets.Count
Set sheet = wbExcel.Worksheets(4)
 
Const deb_client = 1
Const cellule = 3
'parcours du tableau client
fin_client = parcours_tableau(deb_client, cellule)
deb_contrat = fin_client + 4
'parcours du tableau contrat
fin_contrat = parcours_tableau(deb_contrat, cellule)
deb_support = fin_contrat + 4
'parcours du tableau support
fin_support = parcours_tableau(deb_support, cellule)

Voici mon module

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
36
37
 
'Déclaration des variables
Public appExcel As Excel.Application 'Application Excel
Public wbExcel As Excel.Workbook 'Classeur Excel
Public wsExcel As Excel.Worksheet
'Feuille Excel
Public sheet As Excel.Worksheet
 
 
Public nomtemp As String
Public chemin_temp As String
Public chemin_temp_port As String
 
Public exldoc2 As Object
Public exlapp2 As Object
 
Private Declare Function shellexecute Lib "shell32.Dll" (ByVal hwnd As Long, ByVal lpoperation As String, ByVal lpfile As String, lpparameters As String, ByVal lpdirectory As String, ByVal nshowcmd As Long) As Long
 
Public Sub ajout_feuille(nom_feuille As String)
Dim nb As Integer
'calcul le nombre de feuille avant d'ajouter
nb = Sheets.Count
'ajout feuille
Sheets.Add
Sheets("Feuil" & nb).Select
Sheets("Feuil" & nb).Name = nom_feuille
 
End Sub
 
Function parcours_tableau(ByVal deb As Integer, ByVal cellule) As Integer
 
'parcours du tableau client
While (sheet.Cells(deb, cellule).Value <> "")
    deb = deb + 1
Wend
parcours_tableau = deb
End Function

J'hésite notamment à rajouter une boucle FOR pour mes trois check.
Est-ce rentable ? Est-ce justifié ?
Merci d'avance pour vos commentaires.

10/08/2006, 20h58
SfJ5Rpw8
avec :
Code:

1 2 Dim i, fin_client, deb_contrat, fin_contrat, deb_support, fin_support, nb_feuille As Integer
il n'y as que nb_feuille qui est un integer les autres sont des variants...
il faut faire
Code:

1 2 3 4 5 6 7 Dim i As integer , fin_client As integer, deb_contrat As integer ... '..ou Dim i As integer Dim fin_client As integer ....
eu.... avant d'aller plus loin il fonctionne ton code ..? s'il y as des problémes utilise une discussion par problémes........
11/08/2006, 11h43
startout

Oui mon code fonctionne. J'obtiens l'effet désiré.
Je suis simplement en train de l'optimiser.

En ce qui concerne fin_client As integer, deb_contrat, etc ce sont bien des entiers.
Je m'en sers pour garder en mémoire la ligne où commence le tableau client, la ligne où commence le tableau contrat, etc...

Sinon en ce qui concerne mes trois check que je pourrais intégrer dans une boucle.
Vous en pensez koi ?
J'ai vu en structure de données qu'il fallait éviter ce genre de structure. Une boucle qui se trouve dans une autre boucle. Complexité trop importante.
Merci de vos commentaires.
11/08/2006, 11h49
jmfmarques

Citation:

Sinon en ce qui concerne mes trois check que je pourrais intégrer dans une boucle

Ce que l'on pense ?
Que tu pourrais utiliser un groupe indexé et jouer avec l'index, directement ...
11/08/2006, 11h54
startout

J'y est pensé effectivement.
Cela va améliorer la rapidité d'après vous ?
11/08/2006, 12h12
jmfmarques

oui... mais aussi le reste.

Voilà j'ai terminé l'optimisation de cette partie du code
ce qui donne pour mes checkbox :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
etat(1)="en cours client"
etat(2)="en cours client par contrat"
etat(3)="en cours client par support"
 
For i=1 to 3
   'si l'utilisateur souhaite importer l'en cours par client
    If Check(i).Value = 1 Then
       'Test pour savoir si les feuilles existe
       If feuille_existe(etat(i)) = False Then
           Module1.ajout_feuille (etat(i))
       End If
   End If
next i

Conclusion un gain de ligne et un modularité accru.
Merci pour tous.
Et j'oublie pas le tag résolu. 8-)

13/08/2006, 09h55
jmfmarques

Citation:

etat(1)="en cours client"
etat(2)="en cours client par contrat"
etat(3)="en cours client par support"

C'est bien, mais...
Tu aurais pu éviter un tableau et jpuer avec le seul index de la checkbox cochée
Intéresse-toi à la fonction Choose, parfaitement adaptée à ce cas de figure ;)

(Juste pour la beauté du geste et pour apprendre quelque chose, puisque l'occasion t'en est donnée et que tu as manifesté le voeu honorable de perfectionner ton code )

Tiens : essaye :

1) si l'index de tes checkboxes commence à 0 :
Code:

1 2 3 Private Sub Check1_Click(Index As Integer) MsgBox Choose(Index + 1, "toto", "titi", "tata") End Sub
2) s'il commence à 1 :
Code:

1 2 3 Private Sub Check1_Click(Index As Integer) MsgBox Choose(Index , "toto", "titi", "tata") End Sub
Tu vois ?

13/08/2006, 15h50
AlainTech

startout, as tu bien compris ce que bbil a essayé de te dire?
Avec cette syntaxe:

Code:

Dim i, fin_client, deb_contrat, fin_contrat, deb_support, fin_support, nb_feuille As Integer

seule nb_feuille est déclaré comme Integer.
Les autres, n'étant pas suivis d'un As, sont déclarés comme Variant.
Ca ne porte pas vraiment à conséquence mais comme tu nous demandes une critique, j'en conclus que tu veux apprendre.

Relis bien le message de bbil qui te dit comment il faut faire les déclarations de variables.
14/08/2006, 11h00
startout
Citation:
Envoyé par jmfmarques

Tiens : essaye :

1) si l'index de tes checkboxes commence à 0 :

Code:

1 2 3 Private Sub Check1_Click(Index As Integer) MsgBox Choose(Index + 1, "toto", "titi", "tata") End Sub

2) s'il commence à 1 :

Code:

1 2 3 Private Sub Check1_Click(Index As Integer) MsgBox Choose(Index , "toto", "titi", "tata") End Sub

Tu vois ?
Merci, merci beaucoup pour la fonction Choose.
Je ne la connaissais pas. Je vais tester cela de suite et remplacer mes tableaux :P

Startout content.
14/08/2006, 11h03
startout

Citation:

Envoyé par AlainTech

startout, as tu bien compris ce que bbil a essayé de te dire?
Avec cette syntaxe:

Code:

Dim i, fin_client, deb_contrat, fin_contrat, deb_support, fin_support, nb_feuille As Integer

seule nb_feuille est déclaré comme Integer.
Les autres, n'étant pas suivis d'un As, sont déclarés comme Variant.
Ca ne porte pas vraiment à conséquence mais comme tu nous demandes une critique, j'en conclus que tu veux apprendre.

Relis bien le message de bbil qui te dit comment il faut faire les déclarations de variables.

Effectivemment, je n'avais pas compris ce que m'avais dit bbil.
Méa culpa.
Je dis donc merci à bbil et merci à toi pour avoir reformulé sa requarque.

Startout qui a appris beaucoup de chose aujourd'hui.

J'ai testé la fonction choose :ccool: et c'est vraiment pratique.
ce qui donne :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
 
const a= bidule1
const b= bidule2
const c= bidule3
 
For i=1 to 3
    If Check(i).Value = 1 Then
       If feuille_existe(choose(i,"a","b","c")) = False Then
           Module1.ajout_feuille (choose(i,"a","b","c"))
       End If
   End If
next i

Voilà mon code tout propre.
Merci pour votre aide précieuse