XL 2019 VBA qui fonctionne, souhaite relecture

Dim.Reichart

XLDnaute Occasionnel
Bonsoir,
Comme dit dans le titre, ma macro fonctionne, en tout cas je n'ai pas réussi à déclencher d'erreur
Alors je ne sais pas si ça se fait, mais comme c'est ma première, ça me rassurerait qu'un codeur plus expérimenté prenne le temps de me relire pour vérifier que je n'ai rien oublié et que j'utilise les bonnes fonctions au bon mmoment (et pas celles qui demandent énormément de ressources au PC...).

Il s'agit de la macro "Imprimer", qui se trouve dans le module 1, et est liée à chaque bouton des pages projets.

J'ai essayé de mettre le maximum de commentaires qui me semblaient utiles pour faciliter la relecture et les corrections si besoin, mais je suis prêt à recevoir toute critique constructive.
Merci pour le temps passé à m'aider.
 

Fichiers joints

eriiiic

XLDnaute Barbatruc
Bonjour,

Allez, je me lance.
En gros, c'est pas mal. on voit que tu y a mis du soin.

VB:
    Set Lig = Cells.Find("", Cells(21, 7), xlValues, xlWhole, xlByColumns, xlNext, False, False)
    If Lig Is Nothing Then Lig = Cells(22, 7)
Peu de chance d'avoir Lig Is Nothing, à moins d'avoir rempli toutes les 1 million de lignes.
En général on part plutôt de la dernière cellule de la colonne et on remonte jusqu'à la 1ère occupée. Qui sera la dernière utilisée, même si qq'un a laissé une ligne vide (contrairement à ta technique). +1 pour avoir la 1ère disponible.
A remplacer donc par :
Code:
Set Lig = Cells(Rows.Count, 7).End(xlUp).Offset(1)
Et met un titre en G21, ce qui permettra de se caler dessus sans ajouter de test
----------------------------------------------------------------------------------


Code:
    'Définir la taille de la plage d'origine
        C = 16
        Do While IsEmpty(Cells(C, 7)) = False
                If C = 11 Then
                If MsgBox("Le planning est vide. Voulez-vous imprimer un planning vide?", vbYesNo, "Confirmer") = vbNo Then GoTo Ligne2
                End If
        C = C - 1
        Loop
Pourquoi ne pas compter le nombre de valeurs dans la plage comme tu as fait au-dessus ? Plus efficace qu'une boucle.
--------------------------------------------------------------------------


Code:
    Range("g10:an" & C).Select
    Selection.Copy
    Range("g" & Lig.Row).Select
    'Remplacer la cellule haut gauche par le couple mois-année
    Selection.PasteSpecial Paste:=xlPasteValues, operation:=xlNone, skipblanks:=False, Transpose:=False
Il faut éviter les .Select qui ralentissent inutilement :
Code:
    Range("g10:an" & C).Copy
    'Remplacer la cellule haut gauche par le couple mois-année
    Range("g" & Lig.Row).PasteSpecial Paste:=xlPasteValues, operation:=xlNone, skipblanks:=False, Transpose:=False
Et comme seule la valeur t'intéresse, ça peut se réduire à :
Code:
    Range("g" & Lig.Row).Value = Range("g10:an" & C).Value
Code:
    ActiveCell.FormulaR1C1 = Format(DateValue(Cells(4, 32) & " " & Cells(4, 36)), "mmmm yyyy")
Du coup ça va mal se passer pour ActiveCell. Qui est de toute façon à éviter si possible.
Remplacer par le range.
-------------------------------------------------------------------------------------


Code:
    If Ancien Is Nothing Then
        GoTo Ligne1
    Else
        '...
        'scinder la fenetre en deux pour visualiser les donnéees existantes
        If MsgBox("Le planning existe déjà, voulez-vous le remplacer?", vbYesNo, "Confirmation") = vbYes Then
            '...
            GoTo Ligne1
        Else
            'si non, remettre une seule fenêtre, terminer la macro
            '...
        End If
        GoTo Ligne2
        GoTo Ligne1
    End If
Ligne1:
Pas beau du tout là... On peut (presque) toujours se passer des Goto qui compliquent la lecture, et peuvent rendre un code compliqué à faire évoluer par la suite.
On ne les utiliser que pour les routines de traitement d'erreur où on n'a pas le choix.

Sauf erreur de ma part tu peux remplacer par cette structure :
Code:
    If Not Ancien Is Nothing Then
        '...
        'scinder la fenetre en deux pour visualiser les donnéees existantes
        If MsgBox("Le planning existe déjà, voulez-vous le remplacer?", vbYesNo, "Confirmation") = vbYes Then
            '...
            'GoTo Ligne1 ' devenu inutile
        Else
            'si non, remettre une seule fenêtre, terminer la macro
            '...
        End If
        ' GoTo Ligne2 ' il est où ???
        ' GoTo Ligne1 ' devenu inutile
    End If
' Ligne1: ' devenu inutile
Bon, je m'arrête-là, sinon on va être plusieurs à poster la même chose.
Tu as encore des .Select à nettoyer.
eric

Edit : ah, j'ai trouvé ton Ligne2:
Comme tu es sensé ne plus avoir de .Select tu peux mettre Range("g11").Select au début (est-il bien utile d'ailleurs ?),
et remplacer tes Goto Ligne2 par des Exit Sub
 
Dernière édition:

Dim.Reichart

XLDnaute Occasionnel
Merci beaucoup, ça me donne déjà du boulot pour demain, mais là je vais me coucher.
Je reposterais le fichier à jour une fois ces corrections apportées.
Ok pour les goto et select à éviter, je me note ça dans un coin.
 

Dim.Reichart

XLDnaute Occasionnel
Bonjour @eriiiic , et les autres s'il y a,
Je viens d'intégrer les corrections suggérées, ça fonctionne, sauf:
VB:
Range("g" & Lig.Row).Value = Range("g10:an" & C).Value
qui ne copie que la 1e cellule (Projet(s)) et la remplace la ligne suivante par la date.

Sinon,
'D?finir la taille de la plage d'origine
C = 16

Do Until IsEmpty(Cells(C, 7)) = False
If C = 11 Then
If MsgBox("Le planning est vide. Voulez-vous imprimer un planning vide?", vbYesNo, "Confirmer") = vbNo Then Exit Sub 'abandon de la proc?dure
End If
C = C - 1
Loop
Code:
'Définir la taille de la plage d'origine
        C = 16
        Do Until IsEmpty(Cells(C, 7)) = False
                If C = 11 Then
                If MsgBox("Le planning est vide. Voulez-vous imprimer un planning vide?", vbYesNo, "Confirmer") = vbNo Then Exit Sub 'abandon de la procédure
                End If
        C = C - 1
        Loop
Je compte à partir du bas, parce que avec end(xlup) si le planning est vide, il recopie tout, jusqu'à la ligne des totaux dont je ne veux pas, et en prime, ca me met le bazar quand il s'agit de remplacer un ancien planning.

Et aussi:

Code:
 Set Lig = Cells(Rows.Count, 7).End(xlUp).Offset(1)
Et met un titre en G21, ce qui permettra de se caler dessus sans ajouter de test
Je ne comprend pas ce que tu appelles titre, tu veux dire de nommer la cellule G21 dans le gestionnaire de nom?
EDIT: je crois que j'ai compris, tu veux dire nommer une variable = range ("g21")?

Je remet le fichier à jour, je ne modifie pas la version du 1er post, sauf si vous jugez que c'est préférable.
Merci, à plus tard.
 

Fichiers joints

Dernière édition:

eriiiic

XLDnaute Barbatruc
Bonjour,

vite fait avant de partir...
Je ne comprend pas ce que tu appelles titre, tu veux dire de nommer la cellule G21 dans le gestionnaire de nom?
Par mettre un titre je voulais dire mettre qq chose dans cette cellule. Pour que la recherche en partant du bas la trouve si rien d'autre n'a été inscrit en-dessous.
eric
 

Discussions similaires


Haut Bas