1

Intended Result

  • If a row in a table contains any of the listed strings in column L on Sheet1, Then copy the entire row from Sheet1 and paste the row into a duplicate table on Sheet2 (which would be blank at the beginning). (UNINTERESTED, UNRELATED, UNDECIDED, etc...)
  • Then delete the entire row that was transferred from sheet 1.
  • After macro runs, the new transfers should not reset table on Sheet2, rather add rows on the pre-existing lines. This document would be utilized over months.

Variables

  • Sheet1 is named Pipeline_Input
  • Sheet2 is named Closed_Sheet
  • Sheet1 table is named tblData
  • Sheet2 table is named tblClosed

Images

  • Image 1 is the code with error Code
  • Image 2 is Sheet 1 with some picture explanation Sheet1
  • Image 3 is Sheet 2 with some picture explanation Sheet2

Current Result Run-time error '1004': Application-defined or object-defined error

Sub closedsheet()

Application.ScreenUpdating = False

    Dim Pipeline_input As Worksheet 'where is the data copied from
    Dim Closed_Sheet As Worksheet 'where is the data pasted to
    Dim strPhase() As String
    Dim i As Integer
    Dim intPhaseMax As Integer
    Dim lngLstRow As Long
    Dim rngCell As Range
    Dim finalrow As Integer
    Dim lr As Long 'row counter
    Dim Looper As Integer

    intPhaseMax = 6
    ReDim strPhase(1 To intPhaseMax)

    strPhase(1) = "LOST"
    strPhase(2) = "BAD"
    strPhase(3) = "UNINTERESTED"
    strPhase(4) = "UNRELATED"
    strPhase(5) = "UNDECIDED"
    strPhase(6) = "BUDGET"

    'set variables
    Set Pipeline_input = Sheet1
    Set Closed_Sheet = Sheet2

lr = Range("A" & Rows.Count).End(xlUp).Row

For Looper = LBound(strPhase) To UBound(strPhase)

    For i = lr To 6 Step -1
    Next
        If Not Sheet1.Range("L9:L300" & lngLstRow).Find(strPhase(Looper), lookat:=xlWhole) Is Nothing Then
        Range(Cells(i, 1), Cells(i, 20)).Copy
        Sheet2.Range("A" & Rows.Count).End(3)(2).PasteSpecial xlPasteValues
        Range(Cells(i, 1), Cells(i, 20)).Delete
    End If
Next

Sheet2.Select
Sheet2.columns.AutoFit
Application.CutCopyMode = False
Application.ScreenUpdating = True

End Sub
Kobetron
  • 31
  • 4
  • 2
    You have an unqualified `Range()` and `Cells()` in your next loop. Make sure to put the worksheet name before both of those (or at least reference i.e. `Sheet2.Range(Sheet2.Cells()...)...` Also, why do you have `For` line immediately followed by `Next`? What's the point in that loop? – BruceWayne Nov 03 '17 at 17:23
  • @BruceWayne If you are referring to the `For i = lr To 6 Step -1` , it is because I receive a " Compile error: Expected: end of statement" if I remove the For and Next. I am no longer receiving an error with your fix regarding references to Sheet1 and Sheet2 but the code just doesn't do anything. It seems like it doesn't check the column for my strings – Kobetron Nov 03 '17 at 17:32
  • @Kobetron Your `Next` is in the wrong spot. It should be after your `End If` just below it. – dwirony Nov 03 '17 at 17:34
  • @dwirony Code changed to `For i = lr To 6 Step -1 If Not Sheet1.Range("L9:L300" & lngLstRow).Find(strPhase(Looper), lookat:=xlWhole) Is Nothing Then Sheet1.Range(Cells(i, 1), Cells(i, 20)).Copy Sheet2.Range("A" & Rows.Count).End(3)(2).PasteSpecial xlPasteValues Sheet1.Range(Cells(i, 1), Cells(i, 20)).Delete End If Next Sheet2.Select Sheet2.columns.AutoFit Application.CutCopyMode = False Application.ScreenUpdating = True` New error received as "Compile error: For without Next" while highlighting the Sub closedsheet() – Kobetron Nov 03 '17 at 17:37

1 Answers1

0

Okay, there were a plethora of issues with the code you posted, but I decided to help you out here - Notice a few things - There's no copying and pasting here - we're just transferring data.

Secondly, use easy to understand variables. lr and lngLastRow can't be distinguished from one another, so classify them by which worksheet you're getting that value from.

We create an array in one fell swoop here - Just declare a variant and place our values in. ARRAYS (TYPICALLY) START AT ZERO, NOT ONE, so our loop starts at 0 :). Again, this is what's known as best practice...

I swapped out Looper for j. Again, keep. it. simple!

EDIT: I tested this code out on a simulated workbook and it worked fine - should run into no issues for you either.

EDIT2: Also, always use Option Explicit!

Option Explicit
Sub closedsheet()
Application.ScreenUpdating = False

Dim Pipeline_Input As Worksheet 'source sheet
Dim Closed_Sheet As Worksheet 'destination sheet

Dim i As Long, j As Long, CSlastrow As Long, PIlastrow As Long

Dim strPhase As Variant

'Here we create our array
strPhase = Array("LOST", "BAD", "UNINTERESTED", "UNRELATED", "UNDECIDED", "BUDGET")

'Assign worksheets
Set Pipeline_Input = ActiveWorkbook.Worksheets("Pipeline_Input")
Set Closed_Sheet = ActiveWorkbook.Worksheets("Closed_Sheet")

PIlastrow = Pipeline_Input.Range("A" & Rows.Count).End(xlUp).Row

For j = 0 To UBound(strPhase)
    For i = PIlastrow To 6 Step -1
        If Pipeline_Input.Range("L" & i).Value = strPhase(j) Then

            'Refresh lastrow value
            CSlastrow = Closed_Sheet.Range("A" & Rows.Count).End(xlUp).Row

            'Transfer data
            Closed_Sheet.Range("A" & CSlastrow + 1 & ":S" & CSlastrow + 1).Value = _
            Pipeline_Input.Range("A" & i & ":S" & i).Value

            'Delete the line
            Pipeline_Input.Range("A" & i & ":S" & i).EntireRow.Delete

        End If
    Next i
Next j

Closed_Sheet.Select
Closed_Sheet.Columns.AutoFit
Application.ScreenUpdating = True
End Sub
dwirony
  • 5,487
  • 3
  • 21
  • 43
  • Arrays don't always `START AT ZERO` in this case yes but assigning values directly from a range, no, it starts at 1. And it is also a setting, you can start you arrays at 1 if you want. – Scott Craner Nov 03 '17 at 17:55
  • @ScottCraner Yes you're right. I just thought it was silly that he used `ReDim` and chose 1-6, when I would think best practice would be to `ReDim` 0 to 5. – dwirony Nov 03 '17 at 17:56
  • @dwirony Incredible. I have another worksheet change that is active in my sheet that is causing issues but I will remake this with this first and see if I can get the two to coexist. I'll give this a resolved mark after a tad more fiddling. I really appreciate it! – Kobetron Nov 03 '17 at 18:31
  • @dwirony Can confirm that the code provided does transfer data when ran but it does not add it to the table. It pushes the data to the bottom but it would ultimately be needed inside the tblClosed. Any easy change to add it to the Table? In addition, do you think this could be assigned to a button as well? Let me know if i should create a separate question as this is technically answered. – Kobetron Nov 03 '17 at 19:55
  • @Kobetron What do you mean "Inside the tblClosed"? Not sure what that is - – dwirony Nov 03 '17 at 20:06
  • @dwirony When the script is ran, the data gets added to a normal cell that is not associated to the table. So for instance, if my table on sheet2 is from row 8-15, and I transfer the information from sheet 1, the script will bring the data over but put it on line 16 (which is out of the table). – Kobetron Nov 03 '17 at 20:35
  • So you want it to override the data in rows 8-15? – dwirony Nov 03 '17 at 21:28
  • @dwirony my table will have a varied number of rows over time so I am trying to get it so that the transferred rows from sheet 1 to sheet 2 will INSERT the rows (so nothing is deleted in sheet 2) into the tblClosed. This is needed because there will be slicers attached to the table on sheet 2. Sorry for the confusing verbiage. – Kobetron Nov 03 '17 at 21:58
  • @dwirony Instead of equaling a cell (transferring) could we select the range based on our parameter, then copy, and then insert copied rows to table? – Kobetron Nov 06 '17 at 17:50