Why does my macro stop copying all row data after a certain point?

Question

I have coded an Excel macro that imports data from a csv file and then copies rows based on a checked value and places the parsed data into separate sheets. I am checking for 12 values and the first 9 work, but once it gets to 10, 11, and 12 the macro only copies 1 row. Is this a problem with my code or is this a limitation of excel? If it is my code, what should I adjust?

Top module:

Sub Import_Parse_Refresh()
'Import Data CSV
    Call GetCSVList

'Parse Data Based on Report ID
    Call Data_Parse_All

'Refresh Each Pivot Table
    Call TableRefresh

'Delete Imported_Data that was created during the import
    Sheets("Imported_Data").Delete
    Sheets("Begin").Delete

'Save File As
    Call SaveFile

End Sub

Data_Parse_All module:

Sub Data_Parse_All()

    Call Data_Parse_1
    Call Data_Parse_2
    Call Data_Parse_3
    Call Data_Parse_4
    Call Data_Parse_5
    Call Data_Parse_6
    Call Data_Parse_7
    Call Data_Parse_8
    Call Data_Parse_9
    Call Data_Parse_10
    Call Data_Parse_11
    Call Data_Parse_12

End Sub

Data_Parse_9-this code is used for all 12 Data_Parse_# modules but only 1 through 9 work correctly:

Sub Data_Parse_9()
'
    Sheets("Imported_Data").Select
    RowCount = Cells(Cells.Rows.Count, "I").End(xlUp).Row
    For i = 1 To RowCount
        Range("I" & i).Select
        check_value = ActiveCell
        If check_value = "9" Then
            ActiveCell.EntireRow.Cut
            Sheets("Report 9").Select
            RowCount = Cells(Cells.Rows.Count, "A").End(xlUp).Row
            Range("A" & RowCount + 1).Select
            ActiveSheet.Paste
            Sheets("Imported_Data").Select
        End If
    Next

End Sub

Data_Parse_10 -The code is the same but this is when only one row is copied

Sub Data_Parse_10()
'
' Macro1_Data Macro
'
'assuming the data is in sheet1
    Sheets("Imported_Data").Select
    RowCount = Cells(Cells.Rows.Count, "I").End(xlUp).Row
    For i = 1 To RowCount
        Range("I" & i).Select
        check_value = ActiveCell
        If check_value = "10" Then
            ActiveCell.EntireRow.Cut
            Sheets("Report 10").Select
            RowCount = Cells(Cells.Rows.Count, "A").End(xlUp).Row
            Range("A" & RowCount + 1).Select
            ActiveSheet.Paste
            Sheets("Imported_Data").Select
        End If
    Next

End Sub

You can condense all of this into 1 loop. When you see repeating code, think loop. Looks like you will want to step the `check_value` by 1 and the sheet name `Report` by 1. Then you just need 1 macro — urdearboy, Feb 27 '19 at 19:11
Also, [try to avoid Select/Activate](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba). — Mathieu Guindon, Feb 27 '19 at 19:13
Also, nothing wrong with multiple macros as limiting scopes can go a long way in departmentalizing code. However, this is the same task, so I would suggest a merge — urdearboy, Feb 27 '19 at 19:16
@urdearboy - Absolutely I think OP should look in to this. If they even want to change the column they're looking in for, say `RowCount`, they'll have to edit 10 separate macros. OP, I highly suggest you look in to Passing a Parameter to a Subroutine. — BruceWayne, Feb 27 '19 at 19:20
@brucewayne This was my bad. I will edit and remove my question. — Prof. Falken, Feb 27 '19 at 19:29

Tate Garringer · Accepted Answer · 2019-03-01T14:06:58.583

I would bet that the code returning undesirable results has something to do with resetting the To RowCount parameter of your For-Next loop to a different, potentially smaller, value in the middle of the loop. For instance, if Column A of Sheets("Report 10") is empty, then RowCount will be reset to 1, thus exiting the loop after the first iteration. Additionally, as @urdearboy mentioned, you could consolidate this into one dynamic loop. I would try something like

Sub Data_Parse_All()
    Dim i As Long
    Dim Rowcount As Long
    Dim PasteRow As Long

    With Sheets("Imported_Data")
        Rowcount = .Cells(.Cells.Rows.Count, "I").End(xlUp).Row
        For i = 1 To Rowcount
            If .Range("I" & i) >= 1 And .Range("I" & i) <= 12 Then
                PasteRow = Sheets("Report " & .Range("I" & i)).Range("I" & Rows.Count).End(xlUp).Row + 1
                .Range("I" & i).EntireRow.Cut Sheets("Report " & .Range("I" & i)).Range("A" & PasteRow)
            End If
        Next i
    End With
End Sub

This example code did the help me clean up the code and I wanted to thank you for that. While testing your code to see if it fixed the over all problem I noticed something about the data that your code pointed me to. `.Range("A" & Rows.Count)` line needs to have data in cell `A1` which, once it try to check value for 10, 11, and 12, there is no data in that cell thus terminating the code after it copied one line. I change this to `.Range("I" & Rows.Count)` since this is the checked value and will always have data in the cell. — Prof. Falken, Mar 01 '19 at 13:28

BruceWayne · Answer 2 · 2019-02-27T19:45:10.073

2

I think you can collapse all your Data_Parse_# subs in to a single sub. This will take the check_value and use that to get the destination worksheet.

Sub data_Parse()
Dim rowCount As Long, i As Long, newRowCount As Long
Dim check_value As String
Dim destSheet As Worksheet


With ThisWorkbook.Sheets("Imported_Data")
    rowCount = .Cells(Rows.Count, "I").End(xlUp).row
    For i = 1 To rowCount
        check_value = .Cells(i, "I").Value
        Set destSheet = ThisWorkbook.Sheets("Report " & check_value)
        newRowCount = destSheet.Cells(Rows.Count, "A").End(xlUp).row
        .Rows(i).EntireRow.Cut
        destSheet.Range("A" & newRowCount + 1).Paste
        Application.CutCopyMode = False
    Next i
End With ' .Sheets("Imported_Data")
End Sub

(Edit: I know with .Delete, you should go backwards when looping (For i = rowCount to 1 Step -1), but I'm not sure if that's required with .Cut, so just make sure all rows are accounted for.)

edited Feb 27 '19 at 19:45

answered Feb 27 '19 at 19:29

BruceWayne

22,923
15
65
110

Alternate is to add a parameter to the `Sub` call, and put the loop in the main code. – AJD Feb 27 '19 at 20:15
@AJD - Yeah, that's what I was also working on but kept the part for OP where it's a separate self-contained sub. But that's a great suggestion for OP to work on and learn about passing parameters. :) – BruceWayne Feb 27 '19 at 20:46
@BruceWayne I was able to consolidate the code as suggested and I wanted to thank for the example code. This helped a lot! – Prof. Falken Mar 01 '19 at 13:22

Why does my macro stop copying all row data after a certain point?

2 Answers2