0

I tried to copy a set of filtered data from the worksheet "PEP" (Filter: Emetteurs, in column 18) and paste it in the next empty row in the worksheet"Sanctions". I found this text code online and add the section NextRow in order to paste it on the next Empty Row in the "Sanctions" Worksheet. I just started using VBA not long ago, so I tried to use codes online however in this case, I think the problem here is that I cannot define the value of range. I tried to look it up online but no result. Thank you in advance for all your help!

Dim k,j,i As Interger
ActiveWorkbook.Sheets("Sanctions").Select
NextRow=Cells(Range("A"&Rows.Count).End(xlUp).Row+1,1

k=1
With sheets("PEP")
 T=.Range("A1:AS"&.Cells(Rows.Count,1).End(xlUp).Row)
 .Rows(1).Copy Sheets("Sanctions").Range("NextRow")
End With

For i=2 To UBound(T,1)
 If T(i,18)="Emetteurs" Then
   k=k+1
   For j =1 to UBound(T,2)
    T(k,j)=T(i,j)
   Next j 
 End If

Next i
With Sheets("Sanctions")
 Application.ScreenUpdating=False
 .Range("NextRow").Resiwe(k,UBound(T,2))=T.Offset(1)
 .Columns.AutoFit
 Application.ScreenUpdating=True
End With
End Sub

And if possible I would also like to find a solution to remove the header once I do the Copy-Paste process.

braX
  • 11,506
  • 5
  • 20
  • 33
Dat
  • 3
  • 1
  • Note that `Dim k,j,i As Interger` defines `i As Integer` bit `k` and `j` as `Variant`. In VBA you need to specify a type for **every** variable, otherwise it is `Variant` by default. Also they must be of type `Long` because Excel has more rows than `Integer` can handle: `Dim k As Long, j As Long, i As Long`. Since [there is no benefit in using `Integer`](https://stackoverflow.com/questions/26409117/why-use-integer-instead-of-long/26409520#26409520) I recommend always to use `Long` instead. – Pᴇʜ Feb 20 '20 at 13:16

1 Answers1

2

Read the code's comments and adjust it to fit your needs.

Some things to begin:

  1. Use option explicit so you don't have unexpected behavior with undefined variables
  2. Always indent your code (see www.rubberduckvba.com a free tool that helps you with that)
  3. Try to separate your logic defining variables and the reusing them

EDIT: Some reviews to your code:

  1. As mentioned by @Pᴇʜ you need to declare each of your variables types.

    This:

    Dim k,j,i As Interger
    

    is the same as declaring:

    Dim k As Variant
    Dim j As Variant
    Dim i As Integer
    

    Side note. You had a typo in Interger

    Which is not what you really want, knowing that all of them are going to store numbers


  1. Declare the objects that you're going to work with. For example, you refer to the sheet Sanctions three times in your code:

    ActiveWorkbook.Sheets("Sanctions")
    

    This can be set once like this:

    Set targetSheet = ThisWorkbook.Worksheets("Sanctions")
    

    And then reused in lines like this:

    With Sheets("Sanctions")
    

    or this:

    Sheets("Sanctions").Range("NextRow")
    

    by writing this:

    With targetSheet
    

    in this way, if you ever need to change it (the person working with your code, or your future you will be really thankful)


  1. Declaring your variables to just letters, makes your code really hard to understand.

    Dim j,k,l
    

    is different when you have:

    Dim lastRow As Long
    Dim lastColumn As Long
    etc.
    

I suggest that you use the key F8 and step through the code, and see the logic behind it. That way you can learn more.


Code:

Public Sub ConditionalRowCopy()

    ' Declare object variables
    Dim sourceSheet As Worksheet
    Dim targetSheet As Worksheet
    Dim cell As Range

    ' Declare other variables
    Dim sourceLastRow As Long
    Dim targetLastRow As Long

    ' Set a reference to the sheets so you can access them later
    Set sourceSheet = ThisWorkbook.Worksheets("PEP")
    Set targetSheet = ThisWorkbook.Worksheets("Sanctions")

    ' Find last row in source sheet based on column "R"
    sourceLastRow = sourceSheet.Cells(sourceSheet.Rows.Count, "R").End(xlUp).Row

    ' Find cell with word "Emetteurs", search in column R)
    For Each cell In sourceSheet.Range("R1:R" & sourceLastRow).Cells

        ' If match
        If cell.Value = "Emetteurs" Then
            ' Find last row in target sheet based on column "A"
            targetLastRow = targetSheet.Cells(targetSheet.Rows.Count, "A").End(xlUp).Row
            ' Copy entire row to next empty row in target sheet
            cell.EntireRow.Copy Destination:=targetSheet.Range("A" & targetLastRow).Offset(RowOffset:=1)
        End If

    Next cell

End Sub

Let me know if it works!

Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
Ricardo Diaz
  • 5,658
  • 2
  • 19
  • 30
  • It worked magnificently! Thank you. Can you tell me where I was wrong? – Dat Feb 20 '20 at 15:27
  • thank you! I see my errors, I will learn a lot from this! Thanks again for your help! – Dat Feb 20 '20 at 16:35
  • 2
    This answer has valuable advice for a new programmer, and has saved a confused question from being possibly closed. – rleir Feb 20 '20 at 18:53