Copying data from a "master" sheet onto multiple "detail" sheets

Question

I'm new to using VBA. What I'm working on is to copy details from a master file to multiple sheets within a worksheet.

I'm facing a performance problem when I run my code, my Excel file keeps blinking and not responding.

My code is as follows.

Sub Button1_Click()
Dim lastrow As Long
Dim erow As Long
Dim i As Integer

MsgBox ("Clear details of the destination sheets.")

Worksheets("15 Tax Class").Range("A7:V5000").Clear
Worksheets("14 Long Texts").Range("A7:I5000").Clear
Worksheets("12 UoM").Range("A7:V5000").Clear
Worksheets("11 Description").Range("A7:E5000").Clear
Worksheets("09 Sales").Range("A7:AS5000").Clear
Worksheets("03 Plant").Range("A7:FI5000").Clear
Worksheets("02 Client").Range("A7:CR5000").Clear
Worksheets("01 Header").Range("A7:U5000").Clear

MsgBox ("Please wait while template is being populated.")

lastrow = Worksheets("Masterfile").Cells(Rows.Count, 1).End(xlUp).Row

For i = 7 To lastrow

'01 Header Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("01 Header").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("01 Header").Cells(erow + 1, 1)

Worksheets("01 Header").Cells(i, 2) = "Z"
Worksheets("01 Header").Cells(i, 3) = "DIEN"
Worksheets("01 Header").Cells(i, 4) = "X"
Worksheets("01 Header").Cells(i, 5) = "X"
Worksheets("01 Header").Cells(i, 6) = "X"
Worksheets("01 Header").Cells(i, 7) = "X"

'02 Client Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("02 Client").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("02 Client").Cells(erow + 1, 1)

Worksheets("Masterfile").Range("D7:D5000").Copy
erow = Worksheets("02 Client").Cells(Rows.Count, 3).End(xlUp).Row
Worksheets("02 Client").Range("C7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("E7:E5000").Copy
erow = Worksheets("02 Client").Cells(Rows.Count, 4).End(xlUp).Row
Worksheets("02 Client").Range("D7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("F7:F5000").Copy
erow = Worksheets("02 Client").Cells(Rows.Count, 5).End(xlUp).Row
Worksheets("02 Client").Range("E7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("G7:G5000").Copy
erow = Worksheets("02 Client").Cells(Rows.Count, 5).End(xlUp).Row
Worksheets("02 Client").Range("F7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("02 Client").Cells(i, 82) = "NORM"

'03  Plant Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("03 Plant").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("03 Plant").Cells(erow + 1, 1)

Worksheets("Masterfile").Range("H7:H5000").Copy
erow = Worksheets("03 Plant").Cells(Rows.Count, 2).End(xlUp).Row
Worksheets("03 Plant").Range("B7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("I7:I5000").Copy
erow = Worksheets("03 Plant").Cells(Rows.Count, 75).End(xlUp).Row
Worksheets("03 Plant").Range("BW7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

'09 Sales Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("09 Sales").Cells(erow + 1, 1)

Worksheets("Masterfile").Range("J7:J5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 2).End(xlUp).Row
Worksheets("09 Sales").Range("B7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("K7:K5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 3).End(xlUp).Row
Worksheets("09 Sales").Range("C7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("L7:L5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 17).End(xlUp).Row
Worksheets("09 Sales").Range("Q7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("O7:O5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 25).End(xlUp).Row
Worksheets("09 Sales").Range("Y7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("P7:P5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 26).End(xlUp).Row
Worksheets("09 Sales").Range("Z7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("Q7:Q5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 27).End(xlUp).Row
Worksheets("09 Sales").Range("AA7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("R7:R5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 27).End(xlUp).Row
Worksheets("09 Sales").Range("AB7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("S7:S5000").Copy
erow = Worksheets("09 Sales").Cells(Rows.Count, 27).End(xlUp).Row
Worksheets("09 Sales").Range("AC7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

'11 Description Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("11 Description").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("11 Description").Cells(erow + 1, 1)

Worksheets("11 Description").Cells(i, 2) = "EN"

Worksheets("Masterfile").Range("B7:B5000").Copy
erow = Worksheets("11 Description").Cells(Rows.Count, 4).End(xlUp).Row
Worksheets("11 Description").Range("D7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False


'12 UoM Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("12 UoM").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("12 UoM").Cells(erow + 1, 1)

Worksheets("Masterfile").Range("L7:L5000").Copy
erow = Worksheets("12 UoM").Cells(Rows.Count, 2).End(xlUp).Row
Worksheets("12 UoM").Range("B7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("M7:M5000").Copy
erow = Worksheets("12 UoM").Cells(Rows.Count, 4).End(xlUp).Row
Worksheets("12 UoM").Range("D7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

Worksheets("Masterfile").Range("N7:N5000").Copy
erow = Worksheets("12 UoM").Cells(Rows.Count, 5).End(xlUp).Row
Worksheets("12 UoM").Range("E7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

'14 Long Texts Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("14 Long Texts").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("14 Long Texts").Cells(erow + 1, 1)
Worksheets("Masterfile").Paste Destination:=Worksheets("14 Long Texts").Cells(erow + 1, 3)

Worksheets("14 Long Texts").Cells(i, 2) = "MATERIAL"
Worksheets("14 Long Texts").Cells(i, 4) = "GRUN"
Worksheets("14 Long Texts").Cells(i, 5) = "EN"

Worksheets("Masterfile").Range("C7:C5000").Copy
erow = Worksheets("14 Long Texts").Cells(Rows.Count, 8).End(xlUp).Row
Worksheets("14 Long Texts").Range("H7").PasteSpecial Paste:=xlPasteValues, SkipBlanks:=False

'15 Tax Class Data
Worksheets("Masterfile").Cells(i, 1).Copy
erow = Worksheets("15 Tax Class").Cells(Rows.Count, 1).End(xlUp).Row
Worksheets("Masterfile").Paste Destination:=Worksheets("15 Tax Class").Cells(erow + 1, 1)

Worksheets("15 Tax Class").Cells(i, 2) = "PH"
Worksheets("15 Tax Class").Cells(i, 4) = "MWST"
Worksheets("15 Tax Class").Cells(i, 5) = "1"

Next i

MsgBox ("Generating of Upload Template already done!")

End Sub

Answer 1

Sub Button1_Click()

I know it's tempting to just double-click the ActiveX control and code everything in the auto-generated event handler procedure, but that's a very bad habit you will need to lose early on. The handler was generated with an explicit Private access modifier, too - it being absent means it was removed, leaving the procedure implicitly public, exposed to whatever caller code that might feel like invoking an event handler procedure... and event handlers are meant to be invoked by an event - like a button being clicked. Don't make event handlers public, they have no business being invoked by anything else, and thus no business ever being Public - explicitly or not.

Dim lastrow As Long
Dim erow As Long
Dim i As Integer

You may have been taught, or have read somewhere, that declaring all your local variables at the top of the procedure was a good idea - it's not. It only serves to lose track of what's declared with what data type, and in a long procedure it makes the maintainer constantly scroll back and forth, especially with many variables... one problem is that this block of declarations inevitably becomes a wall of declarations, and it becomes very hard to track what's used, and that's exactly how code rots and starts stinking.

But there's another problem: lastRow is a 32-bit integer (Long), but i is only a 16-bit integer (Integer), which means the last row could very well be 32,768... and then that would overflow the 16 bits of Integer, and blow up at run-time: that declaration is a ticking time bomb that needs to be fixed: you want i to be As Long. In fact, there's hardly ever any reason to declare anything As Integer - best stay clear of 16-bit data types for a value representing a row number on a worksheet.

MsgBox ("Clear details of the destination sheets.")

Watch out here, these parentheses aren't delimiting the MsgBox function call's argument list: they are redundantly wrapping up the first argument (the string literal expression), and if you were to decide to add a title and/or an icon (i.e. pass a 2nd and/or a 3rd argument), you'd find that you need to specify them after the ) closing parenthesis, or face a compile-time syntax error.

MsgBox ("Clear details of the destination sheets."), "Awkward", vbOkOnly

When you invoke a function without capturing its return value into a local variable, you do not use any parentheses. What you did here, is force VBA to evaluate the string literal as a value, and pass the result of this evaluation as an argument to the function. When the argument is a string literal this has little impact, but these mechanics will bite you in the rear end sooner or later, and wreck everything as soon as you start passing object references between procedures: best not make it a habit.

Worksheets("15 Tax Class").Range("A7:V5000").Clear

This code is implicitly working off whatever the ActiveWorkbook is. If the sheets all exist in ThisWorkbook, then there is no need to ever dereference any of these sheets from any Sheets collection (kudos for using Worksheets though: the Sheets property yields all kinds of objects including Chart, not just Worksheet items, so it's best to use it when dereferencing worksheets). Worksheets that exist in ThisWorkbook (the host document) at compile-time, have a named code module that you can reference anywhere in your project - simply set the sheets' (Name) property to a valid VBA identifier name like TaxClassSheet, and then you can do this:

TaxClassSheet.Range("A7:V5000").Clear

That hard-coded range address is suspicious: why not work out what the actual range is, instead of crossing fingers and hope that'll do?

Of course that means more code... or does it?

lastrow = Worksheets("Masterfile").Cells(Rows.Count, 1).End(xlUp).Row

If you had a function whose only job was to return the last row in the specified column of a specified worksheet...

Private Function GetLastRowIn(ByVal sheet As Worksheet, ByVal columnIndex As Long = 1) As Long
    GetLastRowIn = sheet.Cells(sheet.Rows.Count, columnIndex).End(xlUp).Row
End Function

...then you could easily get the last row for every sheet you could want the last row for, without repeating the logic every time.

Problem is, with everything in a single scope, the last thing you want is a lastRow variable for which what sheet it's from depends where you're at in the procedure.

Variables that have one meaning at one place, and another meaning in another place, are evil. erow is one such variable: its meaning and purpose evolves as the procedure progresses - and that's a tell-tale sign of a missed opportunity, confirmed with every single one of these headings:

'02 Client Data

Whenever you feel the need for a comment to say "this chunk of code does X", stop. Don't do it. Instead, start a new "do X" procedure:

Private Sub DoX()

In this case it could be ProcessClientData, and then there would be another ProcessPlantData procedure, and then another for the other source sheet, and so on, until the "main" public procedure looks like it doesn't do much more than call a bunch of more specialized, smaller procedures: you get an instant bird's eye view of what the macro does, without all the details that aren't relevant to the big picture - the gory details are in the other, lower-abstraction procedures further down the code module.

I want to slip a word about performance too, but it would be wrong to just say "turn off screen updating and automatic calculations" and call the resulting code anywhere near efficient: these tweaks will stop Excel going "(not responding)" and will make everything run much faster too, but it does not make an inefficient algorithm any better.

Also to be considered, is error handling: whenever this global Application state gets toggled, you absolutely want error handling to ensure the state gets reset back to what it was when the procedure was invoked, otherwise problems can be expected:

    On Error GoTo CleanFail
    Application.EnableEvents = False
    Application.ScreenUpdating = False
    Application.Calculation = xlCalculationManual

    '...

CleanExit:
    Application.EnableEvents = False
    Application.ScreenUpdating = False
    Application.Calculation = xlCalculationManual
    Exit Sub
CleanFail:
    MsgBox Err.Description
    Resume CleanExit
End Sub

Note how the error-handling subroutine is only accessible while execution is in an error state: this separation is critical to avoid the spaghettification easily induced by intertwined "happy" and "error" execution paths.

So that alone should help a lot performance-wise, but as I said above it doesn't make your code any more efficient.

But what's wrong with the approach anyway?

Copying the source data Row By Agonizing Row (RBAR) is what's wrong: the i loop shouldn't even exist - you want to copy the whole source range and paste it at its destination once, ideally for all needed columns together, or at least once per contiguous range of cells. In fact, since all we care about is values (no borders, formats, or validations), we don't even need to get the clipboard involved; just assign destinationRange.Value = sourceRange.Value, and done.

Follow the execution of the loop, and consider how many times cells in rows 7 through 5000 are copied and pasted over: you're copying close to 5K cells every time, to paste at row i.

The result is probably not what you are expecting... and it's only after writing all this that I realize that your code very likely doesn't work as intended after all.