0

From Sheet1 and Sheet2, if a cell from B column has "In Progress", then I want to copy that entire row to another Sheet4.
I want to repeat it for all rows of both the sheets.

Sub Demo1()

    Dim wb As Workbook
    Dim ws As Worksheet, sh As Worksheet
    Dim lastrow As Long
    Dim w As Integer
    Dim i As Integer
    
    Set wb = Workbooks(Book1)
    Set ws = Worksheets("Sheet4")
    Set sh = ActiveSheet
    
    For w = 1 To wb.Sheets.Count
    
        For i = 1 To lastrow
        
            If ActiveSheetCells(i, 2).Value = "In Progress" Then
            
            wb.ws.Cells(1, 1).Insert
            Else
            If Cells(i, 2).Value = "" And i < 50 Then
            ActiveCell.Offset(1, 0).Select
            End If
            Cells(i, 2).Value = "" And i > 49
       Next i
    
    Next w
End Sub

Error Message
enter image description here

Sheet 1
enter image description here

Sheet 2
enter image description here

Sheet 3
enter image description here

Community
  • 1
  • 1
Chaitanya Krishna
  • 21
  • 2
  • "It doesn't work" doesn't tell us much. Give the error, the line it occurs, etc. As a start, this line needs a `.`: `ActiveSheet.Cells(i, 2)` – Cyril Oct 05 '22 at 12:54
  • `Set wb = Workbooks(Book1)`... if "Book1" is a label, it needs to have quotations around it; a variable as no quotations. It may even be more appropriate to `Set wb = ThisWorkbook` then `Set ws = wb.Worksheets("Sheet4")`, so you have a book/sheet reference when using `ws`. – Cyril Oct 05 '22 at 12:56
  • Furthermore, you have unqualified `Cells()` references... make sure you're utilizing a `sourceSheet` and `destinationSheet` to keep track of what is happening and where it is happening. – Cyril Oct 05 '22 at 12:57
  • Chaitanya, if you do not want the use the autofilter and want to use your looping method then leave a message here. One of us will open the thread. :) – Siddharth Rout Oct 05 '22 at 13:11
  • Thank You @Cyril for your suggestions. I made those corrections. I'm adding screenshots of my error and sheets in workbook in the question. – Chaitanya Krishna Oct 05 '22 at 16:43
  • @SiddharthRout Thanks for the reply. Yeah, I would like to use looping method itself. – Chaitanya Krishna Oct 05 '22 at 16:54
  • Your error is because of a lack of `End If`, as you're still inside... `If / Else / If / End If` only closes the second `If`-statement. Better use of indenting would help with checking. – Cyril Oct 06 '22 at 12:15

1 Answers1

0

Quick review on your code, based on my comments to the post (untested):

Sub Demo1()
Dim wb As Workbook:  Set wb = Workbooks("Book1")
Dim destinationSheet As Worksheet:  Set destinationSheet = wb.Worksheets("Sheet4")
Dim sourceSheet As Worksheet:  Set sourceSheet = ActiveSheet
With sourceSheet
    Dim lastRowSource As Long:  lastRowSource = .Cells(.Rows.Count, 1).End(xlUp).Row
    Dim w As Long, i As Long
    For w = 1 To wb.Sheets.Count
        For i = 1 To lastRowSource
            If .Cells(i, 2).Value = "In Progress" Then
                destinationSheet.Cells(1, 1).Insert
            Else
                If .Cells(i, 2).Value = "" And i < 50 Then
                    'Why are you Selecting and what are you doing with it?
                    .Cells(i,X).Offset(1, 0).Select 'Change from "activeCell" to an actual cell reference as you don't change the activecell when looping... 
                End If
                Cells(i, 2).Value = "" And i > 49 'Is this supposed to be another If statement?
            End If 'Added
       Next i
    Next w
End With

Don't use Integer, use Long; the prior gets converted within VBA so you can save the processing with using the latter.

Use descriptive variable names so you're not lost in 10 months re-looking at your code, or having someone else look at your code. For the most part, people should be able to understand what's happening without the use of excessive comments.

Do your best to not have a wall of variables. If you can dimension a variable just as it's being used, you're pairing things together and might catch that x as long when you're using it as a string a lot faster.

You have a .Select and nothing happens with that. Additionally, included as a comment, using ActiveCell is probably not what you want... use a direct cell reference. Note that when you loop, VBA will change its references, however it does not physically change its activecell.

You have what appears to be another If statement which does not include any If / Then for the i > 49 bit.

The culprit of your error is the lack of End If, which is now placed with the comment Added.

Cyril
  • 6,448
  • 1
  • 18
  • 31
  • Thanks for your review. I'll keep ypur suggestions in mind. I want to end the for loop and if statement if i>49 & cells(i,2) is blank... That's why I used "End If". I guess, I took End If literally. – Chaitanya Krishna Oct 06 '22 at 12:54
  • @ChaitanyaKrishna it ends *each* if, so you need more than one... there's also [`elseif`](https://learn.microsoft.com/en-us/office/vba/language/concepts/getting-started/using-ifthenelse-statements), depending on the scope you want; however, I prefer `Select Case` over `ElseIf` statements for readability. – Cyril Oct 06 '22 at 13:42