Trying to add variables to an existing code

Question

This Macro was created by my predecessor, and I would like to clean it up to be more efficient.
The Variables are not defined, and I would like to make sure I'm doing this correctly.

The Macro starts with one Workbook open, but opens other Workbooks, pulls the data, and pastes into the first workbook.

Sub DataPaste()

'Turn Off Screen Updates
    Application.ScreenUpdating = False
    Application.DisplayAlerts = False
    Application.Calculation = xlCalculationManual

'Open Standard Data Reports
    Workbooks.Open "O:\Wholesale\Reporting\Market6 Scorecard\Templates\26 Wk Data.csv"

'Copy 26 Wk Data
            Set dWkData = Workbooks("26 Wk Data.csv").Worksheets("26 Wk Data")
            Set dDataPaste = Workbooks("KROGER M6 SCORECARD TEMPLATE.xlsm").Worksheets("COMBINED")
            
            dTemplateLastRow = dDataPaste.Cells(dDataPaste.Rows.Count, "B").End(xlUp).Offset(1).Row
            dCopyLastRow = dWkData.Cells(dWkData.Rows.Count, "A").End(xlUp).Row
            
            dWkData.Range("A18:H" & dCopyLastRow).Copy dDataPaste.Range("B" & dTemplateLastRow)
            dWkData.Range("I18:R" & dCopyLastRow).Copy dDataPaste.Range("L" & dTemplateLastRow)
            
'Add Dates
                dTemplateLastRowb = dDataPaste.Cells(dDataPaste.Rows.Count, "B").End(xlUp).Row
                dTemplateLastRowc = dDataPaste.Cells(dDataPaste.Rows.Count, "B").End(xlUp).Offset(1).Row
                Set dFirstRow = Workbooks("KROGER M6 SCORECARD TEMPLATE.xlsm").Worksheets("COMBINED").Range("A" & cTemplateLastRowc)
                Set dLastRow = Workbooks("KROGER M6 SCORECARD TEMPLATE.xlsm").Worksheets("COMBINED").Range("A" & dTemplateLastRowb)
                Range(dFirstRow, dLastRow).Formula = "=concatenate(""Latest 26 Wks - Ending "",left(right('Weekly Division'!$A$4,24),23))"

'Close Standard Data Reports
    Workbooks("26 Wk Data.csv").Close SaveChanges:=False

'Calculate Workbook
    Calculate

'Save File as Template File
    ActiveWorkbook.Save
        
'Turn on Screen Updates
    Application.ScreenUpdating = True
    Application.DisplayAlerts = True
    Application.Calculation = xlCalculationAutomatic
   
End Sub

I'm assuming something like this??

'Copy 26 Wk Data
    Dim dWkData as Long
    Dim dDataPaste as Long
            Set dWkData = Workbooks("26 Wk Data.csv").Worksheets("26 Wk Data")
            Set dDataPaste = Workbooks("KROGER M6 SCORECARD TEMPLATE.xlsm").Worksheets("COMBINED")
            
   Dim dTemplateLastRow as Long
   Dim dCopyLastRow as Long
            dTemplateLastRow = dDataPaste.Cells(dDataPaste.Rows.Count, "B").End(xlUp).Offset(1).Row
            dCopyLastRow = dWkData.Cells(dWkData.Rows.Count, "A").End(xlUp).Row
            
            dWkData.Range("A18:H" & dCopyLastRow).Copy dDataPaste.Range("B" & dTemplateLastRow)
            dWkData.Range("I18:R" & dCopyLastRow).Copy dDataPaste.Range("L" & dTemplateLastRow)

Answer 1

Here are all of the declarations you need to set up to use the code you supplied:

Dim dWkData As Worksheet, dDataPaste As Worksheet
Dim dTemplateLastRow As Long, dCopyLastRow As Long, dTemplateLastRowb As Long, dTemplateLastRowc As Long
Dim dLastRow As Range, dFirstRow As Range

However, I also notice there appears to be a typo on this line:

Set dFirstRow = Workbooks("KROGER M6 SCORECARD TEMPLATE.xlsm").Worksheets("COMBINED").Range("A" & cTemplateLastRowc)

I think at the end there it should read dTemplateLastRowc not cTemplateLastRowc.

---

As an extra aside, you will often see authors include a hint of the datatype within the variable names, so you might want to consider renaming your variables/objects to something like this:

dWkData -> wsData
dDataPaste -> wsDataPaste
dTemplateLastRow -> lngTemplateLastRow (or lTemplateLastRow)
dCopyLastRow -> lngCopyLastRow (or l..)
dTemplateLastRowb -> lngTemplateLastRowb (or l..)
dLastRow -> rngLastRow

This makes it much easier to remember what you're using the variable/object for when adding new code/making changes.

Answer 2

Just another point if you're trying to solidify the code - if you're using these:

Application.ScreenUpdating = False
Application.DisplayAlerts = False
Application.Calculation = xlCalculationManual

Then it's also good practice to use an error handler to force the routine to switch these back to their defaults at the end of the routine, just in case something goes wrong (Although later versions of Excel seem to fix some of these on error)

You already have them reverting correctly at the end:

Application.ScreenUpdating = True
Application.DisplayAlerts = True
Application.Calculation = xlCalculationAutomatic

But if something broke along the way then these last statements wouldn't get executed and, depending on your Excel version, you might be left with frozen screens, no safety alerts and frozen formulae.

For this reason, if I ever use these I always put a goto error statement just after the initial bit:

Application.ScreenUpdating = False
Application.DisplayAlerts = False
Application.Calculation = xlCalculationManual
On Error GoTo ErrHandler ' tells the runtime if an error occurs to jump to "ErrHandler" line

And then I put that error handler line right above the last bit so it knows where to jump to:

ErrHandler: ' Will jump to here if something goes wrong
Application.ScreenUpdating = True
Application.DisplayAlerts = True
Application.Calculation = xlCalculationAutomatic

Answer 3

Not really:

Change, please:

Dim dWkData as Long
Dim dDataPaste as Long

with:

Dim dWkData as Worksheet
Dim dDataPaste as Worksheet

You can also declare and use. To make the code easy to be read, shorter, especially when you (may) need the workbooks for other worksheets, also. Here, only an example of using it:

Dim WbD as Workbook, WbK as Workbook
Set WbD = Workbooks("26 Wk Data.csv")
Set WbK = Workbooks("KROGER M6 SCORECARD TEMPLATE.xlsm")

Set dWkData = WbD.Worksheets("26 Wk Data")
Set dDataPaste = WbK.Worksheets("COMBINED")