CODEWITHSUNDEEP
excelvbaperformanceoptimizationcopy-paste
Hannah Nev
Hannah Nev

Reputation: 23

copy row from one excel sheet to another based on 2 criteria inputs

I have an excel sheet where the first sheet contains all of the master data which will be around 500 rows and goes across to column R, this sheet is called Overall Sheet. Column O includes a month, and Column P includes a year.

I have another sheet where I would like the data to be copied this is called "Forecast Month". at the top in B1, the month that I would like to be copied in is selected, and in D1, the year is selected. I would like the button to read these two cells and copy in the data from "overall sheet" based on this.

I have written this code as shown below, but for some reason the data is entered into "forecast month" 10 times before adding the next one (also 10 times). I should only have 3 pieces of data in this sheet but instead there is 30, 10 for each.

Also the top 3 lines on each sheet have headings so the data should start writing on row 4 (which it does)

Please can anybody help?? 

Private Sub CommandButton1_Click()
    Dim month As String
    Dim year As String

    Dim c As Range
    Dim d As Range

    Dim k As Integer
    Dim source As Worksheet
    Dim targetforecastmonth As Worksheet

    'change worksheet designations as needed
    Set source = ActiveWorkbook.Worksheets("Overall Sheet")
    Set targetforecastmonth = ActiveWorkbook.Worksheets("Forecast Month")

    targetforecastmonth.Range("A4:Z1000").Clear

    month = ActiveWorkbook.Worksheets("Forecast Month").Range("B1")
    year = ActiveWorkbook.Worksheets("Forecast Month").Range("D1")

    k = 4

    For Each c In source.Range("O4:O1000")
        For Each d In source.Range("P4:P1000")
            If c = month And d = year Then
                source.Rows(c.Row).Copy targetforecastmonth.Rows(k)
                k = k + 1
            End If
        Next d
    Next c
End Sub

Upvotes: 2

Views: 527

Answers (3)

DirtyDeffy
DirtyDeffy

Reputation: 507

You have a nested For Each loop, meaning that you take cell "O4" then loop through cells "P4:P1000" before moving on to cell "O5" and looping through cells "P4:P1000" again and so forth... If for example the cell value of "O4" satisfies the month criteria, then every time the loop through column P finds a cell that satisfies the year criteria, row number 4 will be copied and pasted. Try this instead:

Private Sub CommandButton1_Click()
    Dim month As String
    Dim year As String

    Dim c As Range
    Dim d As Range
    Dim x As Long

    Dim k As Integer
    Dim source As Worksheet
    Dim targetforecastmonth As Worksheet

    'change worksheet designations as needed
    Set source = ActiveWorkbook.Worksheets("Overall Sheet")
    Set targetforecastmonth = ActiveWorkbook.Worksheets("Forecast Month")

    targetforecastmonth.Range("A4:Z1000").Clear

    month = ActiveWorkbook.Worksheets("Forecast Month").Range("B1")
    year = ActiveWorkbook.Worksheets("Forecast Month").Range("D1")

    k = 4
    x = 4

    For Each c In source.Range("O4:O1000")
        Set d = source.Range("P" & x)
        If c.Value = month And d.Value = year Then
            source.Rows(c.Row).Copy targetforecastmonth.Rows(k)
            k = k + 1
        End If
    x = x + 1
    Next c
End Sub

Upvotes: 0

Almantas Bendoraitis
Almantas Bendoraitis

Reputation: 11

It seems wrong logic there is. I suppose you need Expl.: O8, P8 matches B1, D1 So you need only one cycle: 

For Each c In source.Range("O4:O1000")
d = source.Range("P" & k)
If c = month And d = year Then
    source.Rows(c.Row).Copy targetforecastmonth.Rows(k)
End If
k = k + 1
Next c

Upvotes: 1

SINAN NIZAR
SINAN NIZAR

Reputation: 43

Try this, I hope this would help.

Private Sub CommandButton1_Click()
Dim month As String
Dim year As String

Dim c As Range

Dim k As Integer
Dim source As Worksheet
Dim targetforecastmonth As Worksheet

'change worksheet designations as needed
Set source = ActiveWorkbook.Worksheets("Overall Sheet")
Set targetforecastmonth = ActiveWorkbook.Worksheets("Forecast Month")

targetforecastmonth.Range("A4:Z1000").Clear

month = ActiveWorkbook.Worksheets("Forecast Month").Range("B1")
year = ActiveWorkbook.Worksheets("Forecast Month").Range("D1")

k = 4

For Each c In source.Range("O4:O1000")
If c = month And source.Cells(c.Row, 16).Value = year Then
source.Rows(c.Row).Copy targetforecastmonth.Rows(k)
k = k + 1
End If
Next c

End Sub

Upvotes: 0

Related Questions