2

I am attempting to copy filtered data from one sheet to another. It copies everything to the same line.

How do populates all the rows instead of copying them over the same one?

Here is the code I modified:

Private Sub Workbook_Open()
    Dim i, LastRow

    LastRow = Sheets("Scheduled WO's").Range("A" & Rows.Count).End(xlUp).Row
    Sheets("Branden").Range("A2:Q10000").ClearContents

    For i = 2 To LastRow
        If Sheets("Scheduled WO's").Cells(i, "G").Value = "Branden" Then
            Sheets("Scheduled WO's").Cells(i, "G").EntireRow.Copy Destination:=Sheets("Branden").Range("A" & Rows.Count).End(xlUp).Offset(1)
        End If
    Next
End Sub
Community
  • 1
  • 1
ITPRO
  • 21
  • 1
  • 3

2 Answers2

1

you have to take off the statement

Sheets("Branden").Range("A2:Q10000").ClearContents

which clears "Branden" worksheet cells at every opening of the workbook it resides in

furthermore, since your need is filtering, you may want to use Autofilter and avoid looping through cells

Private Sub Workbook_Open()
    With Worksheets("Scheduled WO's")
        With .Range("G1:G" & .Cells(.Rows.Count, 1).End(xlUp).Row)
            .AutoFilter field:=1, Criteria1:="Branden"
            If Application.WorksheetFunction.Subtotal(103, .Cells) - 1 > 0 Then .Offset(1).Resize(.Rows.Count - 1).SpecialCells(XlCellType.xlCellTypeVisible).EntireRow.Copy Worksheets("Branden").Range("A" & Rows.Count).End(xlUp).Offset(1)
        End With
        .AutoFilterMode = False
    End With
End Sub
user3598756
  • 28,893
  • 4
  • 18
  • 28
  • I think clearing that sheet is intentional though, and it also isn't related to his question. – Tyeler Oct 27 '16 at 12:16
  • @Tyeler, to me that _clearing_ is the only possible reason of OP's issue. Removing it his code runs fine: the `Rows.Count` matter isn't effective since all range reference, implicitly, the same `Workbook` so `Rows.Count` isn't an issue – user3598756 Oct 27 '16 at 12:42
  • It only runs that clearing line once, on workbook open. He was saying that the loop was copying each row from the first sheet onto the same row of the second sheet, ultimately leaving him with only one (the last matching row) on his second sheet. The only way I can see that happening is because he's referencing a `Rows.Count` to offset from except that `Rows.Count` never changes, so the offset never changes. Am I wrong? – Tyeler Oct 27 '16 at 13:07
  • That Rows.Count is always followed by a End(xlUp) so that the final offset constantly keeps track of the last not empty cell in the column. As for the rest: it's a Workbook_ Open() event handler so all the code (not only the cells clearance) is executed only once at the workbook opening and gets executed again only at the next workbook opening. Any how we must wait for OP comments – user3598756 Oct 27 '16 at 14:31
  • @Tyeler, I missed to address to you my last comment – user3598756 Oct 27 '16 at 15:21
  • I guess I just don't understand how `Rows.Count` in his code is being read specifically from `Sheets("Branden")` and not the `ActiveSheet`.... I can't find where to answer this for myself. – Tyeler Oct 27 '16 at 16:05
  • `Row.Count` without any explicit worksheet qualification is actually being read from `ActiveSheet`. But the value returned is the same as if it were read from `Sheets("Branden")` or any other worksheet in the same Workbook since it only depends on the Workbook _format_: in the same workbook all worksheets return the same `Rows.Count`. While it can differ between worksheets belonging to different workbooks if these latter have different _formats_ (before and after Excel 2003, if I correctly recall) – user3598756 Oct 27 '16 at 16:58
  • Ah I see it now... I believe this is the second time you've educated me on something I thought I already knew about. Should I delete my answer? After reading his code, I'm not sure I understand why his data is being copied to the same line.. – Tyeler Oct 27 '16 at 17:20
  • 1
    We all are being educated each day. As to your answer it's entirely up to you. Since you pointed out useful issues you may edit it to erase just the part that doesn't actually solve OP issue and leave the rest as good coding practice suggestions. Although this could expose your answer to the downvote by any too "efficient" SO user (not me for sure)... – user3598756 Oct 27 '16 at 17:51
0

Copy Rows Based On Matching Criteria From One Sheet To Another


There's 2 ways we can go about this.

Code 1

The first is sticking with what you were doing, which may or may not be the slower way of accomplishing this (depending on how many cells you're moving through.)

Option Explicit

Private Sub Workbook_Open()
    Dim wsWO As Worksheet: Set wsWO = ThisWorkbook.Sheets("Scheduled WO's")
    Dim wsB As Worksheet: Set wsB = ThisWorkbook.Sheets("Branden")
    Dim LastRow As Long: LastRow = wsWO.Cells(wsWO.Rows.Count, 1).End(xlUp).Row
    Dim i As Long

    wsB.Range("A2:Q10000").ClearContents

    For i = 2 To LastRow
        If wsWO.Cells(i, "G").Value = "Branden" Then _
            wsWO.Cells(i, "G").EntireRow.Copy _
            wsB.Range("A" & wsB.Cells(ws.Rows.Count, 1).End(xlUp).Row + 1)
    Next i
End Sub

Code 2

The other way we can do this is by specifically finding only occurences of "Branden", and copying those rows over.

Option Explicit

Private Sub Workbook_Open()
    Dim wsWO As Worksheet: Set wsWO = ThisWorkbook.Sheets("Scheduled WO's")
    Dim wsB As Worksheet: Set wsB = ThisWorkbook.Sheets("Branden")
    Dim findBranden As Range: Set findBranden = wsWO.Range("G:G") _
        .Find(What:="Branden", LookIn:=xlValues, LookAt:=xlWhole)
    Dim firstResult As String

    wsB.Range("A2:Q10000").ClearContents

    If Not findBranden Is Nothing Then
        firstResult = findBranden.Address
        Do
            findBranden.EntireRow.Copy _
            wsB.Range("A" & wsB.Cells(wsB.Rows.Count, 1).End(xlUp).Row + 1)
            Set findBranden = wsWO.Range("G:G").FindNext(findBranden)
        Loop While Not findBranden Is Nothing And findBranden.Address <> firstResult
    Else: MsgBox "Nothing to move today.", vbInformation, ""
    End If
End Sub

You'll notice there's a couple new things in both codes.

An important one is Option Explicit. Including this at the top of your code module will alert you at compile if you have any variables that aren't declared. This is incredibly useful because it will catch spelling mistakes and the like before your code runs. I dare say all experienced VBA coders use Option Explicit or have Require Variable Declaration turned on in the Tools > Options > Editor menu.

Another very important change was declaring the specific type of variables we are using. In your code, LastRow and i are assumed as Variant types because you never specified their use. It's good practice to be as specific as you can in coding, especially with variable declarations, because it will make troubleshooting your code a lot more simple.

I also declared your worksheets as variables to make the written code smaller, and easier to read.


Literature You May Find Useful

Why Option Explicit?

Why Require Variable Declaration?

Why declare the specific type of variable?


Both methods are viable and easy to manipulate. Let me know if I've managed to help :)

Tyeler
  • 1,088
  • 1
  • 12
  • 26