Discussion :

[iIncoming]

Bonjour,

Le code ci-dessous recopie une feuille vers une autre feuille.
J'aimerais savoir s'il peut être optimisé ? Peut-être que certaines parties ne sont pas utiles ? Si oui, lesquelles ? ErrorCopy ?

Code :

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

1
2
3
4
5
6
7
Function SheetExists(SheetName) As Boolean
    Dim f As Object
    On Error Resume Next
    Set f = Sheets(SheetName)
    If Err = 0 Then SheetExists = True
    Set f = Nothing
End Function

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
Sub SheetCopy()
    Dim st As String
    Dim sh As Worksheet
    Dim shDestination As Worksheet
    Set sh = ActiveSheet
    st = InputBox("Nom de la nouvelle feuille : (Exemple : NOM DE FAMILLE)")
    If st <> "" Then
        If Not SheetExists(st) Then
            On Error GoTo ErrorCopy
            sh.Copy after:=sh
            Set shDestination = ActiveSheet
            shDestination.Name = st
        Else
            MsgBox "La feuille " & st & " existe déjà !"
        End If
    End If
Exit Sub
ErrorCopy:
     MsgBox "Erreur : " & Err.Number & " " & Err.Description & vbCrLf & "Sur copie " & sh.Name & " vers feuille " & st, vbCritical
End Sub

Merci pour votre aide !

[patricktoulon]

Bonjour

tu pourrais commencer par éviter de te servir de on error resume next

surtout quand tu te sert pas de on error goto 0
a la fin (charge mémoire )

je ferait ceci: a ta place

Code :

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

1
2
3
4
5
Function SheetExists(SheetName) As Boolean
    for each sh in worksheets
if sh.name=sheetname then sheetexists=true
next 
End Function

ca c'est juste pour commencer je regarde le reste
Au plaisir

Re pour le reste un debut deja regarde les commentaires

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
Sub SheetCopy()
    Dim st As String
    Dim sh As Worksheet
    Dim shDestination As Worksheet
    Set sh = ActiveSheet
    st = InputBox("Nom de la nouvelle feuille : (Exemple : NOM DE FAMILLE)")
    If st <> "" Then
        If Not SheetExists(st) Then
            On Error GoTo ErrorCopy' je ne vois pas pourquoi tu veux gérer une eventuelle erreur puisque la seule condition c'est "st" si il existe ou pas 
            sh.Copy after:=sh
            Set shDestination = ActiveSheet
            shDestination.Name = st
        Else
            MsgBox "La feuille " & st & " existe déjà !"
        End If
    End If
Exit Sub
ErrorCopy:
     MsgBox "Erreur : " & Err.Number & " " & Err.Description & vbCrLf & "Sur copie " & sh.Name & " vers feuille " & st, vbCritical
End Sub

moi j'aurais fait plutot ca :

Code :

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

1
2
set newsheet=sh.Copy after:=activesheet
newsheet.name=st

[Marc-L]

Bonjour iIncoming.

L'utilisation de On Error Resume Next au sein d'une micro fonction pour un objet pouvant déclencher une erreur ne me gêne pas du tout
car, comme indiqué dans l'aide en ligne, la ressource est automatiquement désactivée et donc libérée en fin de fonction …

Cette fonction peut être simplifiée par exemple ainsi :

Code :

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

1
2
3
4
Function SheetExists(SheetName) As Boolean
         On Error Resume Next
         SheetExists = Sheets(SheetName).Index > 0
End Function

Quant à la routine de gestion d'erreurs de la procédure SheetCopy, elle ne me gêne pas non plus
car dans le cas d'un classeur en lecture seule ou avec sa structure protégée, une erreur serait évidemment déclenchée !
Cela peut se contourner via les propriétés du classeur ReadOnly et ProtectStructure …

Par contre les lignes n°11 & 12 peuvent (doivent !) être optimisées
car à quoi bon définir une variable objet pour ne l'utiliser qu'une seule fois !?

Autant réécrire ces lignes en une seule : ActiveSheet.Name = st

_______________________________________________________________________________

Merci de cliquer sur pour chaque message ayant aidé puis sur pour clore cette discussion …

[iIncoming]

En fait la gestion des erreurs me permet d'afficher des explications au lieu du code 400. Notamment lorsque je rentre @ ou } comme nom d'onglet.

Quant à :

Code :

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

1
2
set newsheet=sh.Copy after:=activesheet
newsheet.name=st

Je ne vois pas trop pourquoi et où mettre ceci en oeuvre ?

[patricktoulon]

Re

ouais si tu veux moi je ne vois pas l'utilité d'utiliser la gestion d'erreur
puisque c'est le active sheets qui est copié

et le test sheetexist teste si le nom n'existe pas déjà

je ne vois pas ou il peut y avoir un erreur car des que l'on ouvre un fichier c'est au moins le sheets(1) qui est activé

enfin moi ce que j'en dis

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
 
Function SheetExists(SheetName) As Boolean
        SheetExists = False' au cas ou on utilise la fonction dans une meme instanciation d'un fichier 
        For Each sh In Worksheets
        If sh.Name = SheetName Then SheetExists = True
     Next
    End Function
 
Sub SheetCopy()
    Dim st As String
        st = InputBox("Nom de la nouvelle feuille : (Exemple : NOM DE FAMILLE)")
    If st <> "" Then
        If Not SheetExists(st) Then
                    ActiveSheet.Copy after:=ActiveSheet
            ActiveSheet.Name = st
        Else
            MsgBox "La feuille " & st & " existe déjà !"
        End If
    End If
st = ""
End Sub

Au plaisir

[iIncoming]

Du coup votre proposition d'optimisation est la bonne ? Vous pensez que j'ai intérêt à l'utiliser au lieu de la version initiale ? Comment puis-je en fin de compte savoir si tel ou tel code est "meilleur" au niveau de la propreté du code et de la gestion de la mémoire qu'un autre ?

Merci pour vos éclaircissements !

[patricktoulon]

Bonjour
les deux solutions sont bonnes ma version supprime la gestion inutile des erreurs

on peut mesurer l'efficacité en terme de durrée d'excecution en metant un test sur sur le temps
Mais sincerement je n'en vois pas l'utilité
Arrete de te torturer
Au plaisir