Worksheet that lists a person's name

Question

I have a worksheet that lists a person's name (column A; many duplicate names are in this column) with associated data (columns B through G). I have code below that takes this list of a ~ 1000 rows and then:

1. Copies and pastes each row three times (to create four identical rows for each entry).
2. Loops through the now ~4000 rows and creates a new worksheet for each person. As there are many duplicate names in column A this only creates a handful of new worksheets.

The thing is, it runs but runs quite slowly (and I receive the Excel not responding warning at times). Is there anything to clean this up to make it more efficient? And after this I run another macro to save the new worksheets to a new workbook. Would it be faster to do that with code here?

Sub Split_Data()

    Const NameCol = "A"
    Const HeaderRow = 1
    Const FirstRow = 2
    Dim SrcSheet As Worksheet
    Dim TrgSheet As Worksheet
    Dim SrcRow As Long
    Dim LastRow As Long
    Dim TrgRow As Long
    Dim person As String
    Dim lRow As Long
    Dim RepeatFactor As Variant

'Optimize Macro Speed
    Application.ScreenUpdating = False
    Application.EnableEvents = False
    Application.Calculation = xlCalculationManual

'Copy each row and paste three times
    lRow = 2
    Do While (Cells(lRow, "B") <> "")
        RepeatFactor = 4

        Range(Cells(lRow, "A"), Cells(lRow, "G")).Copy
        Range(Cells(lRow + 1, "A"), Cells(lRow + RepeatFactor - 1, "G")).Select

        Selection.Insert Shift:=xlDown
        lRow = lRow + RepeatFactor - 1    
        lRow = lRow + 1
    Loop

    Set SrcSheet = ActiveSheet
    LastRow = SrcSheet.Cells(SrcSheet.Rows.Count, NameCol).End(xlUp).Row
    For SrcRow = FirstRow To LastRow
        person = SrcSheet.Cells(SrcRow, NameCol).Value
        Set TrgSheet = Nothing
        On Error Resume Next
        Set TrgSheet = Worksheets(person)
        On Error GoTo 0
        If TrgSheet Is Nothing Then
            Set TrgSheet = Worksheets.Add(After:=Worksheets(Worksheets.Count))
            TrgSheet.Name = person
            SrcSheet.Rows(HeaderRow).Copy Destination:=TrgSheet.Rows(HeaderRow)
        End If
        TrgRow = TrgSheet.Cells(TrgSheet.Rows.Count, NameCol).End(xlUp).Row + 1
        SrcSheet.Rows(SrcRow).Copy Destination:=TrgSheet.Rows(TrgRow)
    Next SrcRow

ResetSettings:
  'Reset Macro Optimization Settings
    Application.EnableEvents = True
    Application.Calculation = xlCalculationAutomatic
    Application.ScreenUpdating = True

End Sub

Answer 1

Here are my thoughts ( I'm using a tablet and so cannot test my suggestions). You are already turning off screen updating and calculations which is good.

1. Avoid using Select where possible - there are plenty of examples of this.
2. The Insert in your code is probably a very slow process because Excel does a lot of work. Why not simply copy the entire data in the columns you want and then paste four times onto another sheet? No need for the Insert or the 1,000 copy & pastes.
3. Testing each row for the person's name to see if a worksheet exists and then copying just that row can also be changed. Why not sort the data by name, then run down the data keeping track of start row of current name and looking to see when the name changes? Then you copy the data from the start row up to the row before the current one. That way you could do just one copy & paste per name into the named worksheet.

Answer 2

Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names. You seem to be using some hungarian notation and Variable names - give your variables meaningful names:

Const NAME_COLUMN As String = "A"
Const HEADER_ROW As Long = 1
Const FIRST_ROW As Long = 2
Dim sourceSheet As Worksheet
Dim targetSheet As Worksheet
Dim sourceRowIndex As Long
Dim lastRow As Long
Dim targetRow As Long
Dim person As String
Dim rowCounter As Long
Dim repeatFactor As Variant

---

Comments and labels

I don't see a need for the label ResetSettings:. Also your comments aren't needed - Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.

---

Select and copy

Be sure to avoid things like .Select - it just slows the code down by needing to fiddle with the spreadsheet while doing everything else behind the scenes. There's a good question on StackOverflow addressing this.

You can completely avoid the copy

sourceSheet.Rows(sourceRowIndex).Copy Destination:=targetSheet.Rows(targetRow)

By doing this instead:

targetSheet.Rows(targetRow) = sourceSheet.Rows(sourceRowIndex).Value

---

This is overly complicated -

Do While (Cells(rowCounter, "B") <> "")

You can use

Do While Not IsEmpty(Cells(rowCounter, "B"))

What if activesheet isn't your target?

Set sourceSheet = ActiveSheet

Try to be more explicit.

---

Error

This

    On Error Resume Next
    Set targetSheet = Worksheets(person)
    On Error GoTo 0
    If targetSheet Is Nothing Then

Has a better way

If Not WorksheetExists(person) then
    Set targetSheet = Worksheets.Add(After:=Worksheets(Worksheets.Count))
    targetSheet.Name = person
    targetSheet.Rows(HEADER_ROW) = sourceSheet.Rows(HEADER_ROW).Value
 End If

Function WorksheetExists(sName As String) As Boolean
    WorksheetExists = Evaluate("ISREF('" & sName & "'!A1)")
End Function

---

Duplicating Rows

There's an easier way to duplicate your rows as well

Sub DuplicateRows()
    Dim lastRow As Long
    lastRow = Sheet1.Cells(Rows.Count, 1).End(xlUp).Row
    Dim i As Long
    Dim j As Long
    For i = lastRow To 1 Step -1
        For j = 1 To 3
            Sheet1.Rows(i).Insert shift:=xlDown
            Sheet1.Rows(i) = Sheet1.Rows(i + 1).Value
        Next
    Next
End Sub

However, why don't you duplicate them once they are already moved to the new sheet, that way you iterate over 1000 instead of 4000?