Cfrgtkky

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

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

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

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