squar_o
squar_o

Reputation: 567

Excel vba reverse for loop stepping beyond end value

I am debugging the code below. when I loop through it, I notice that row_j has a value of 1, despite For row_j = LastRow_date_new To 2 Step -1.

I want the minimum row_j value to be 2 as there is no data in row 1. The values in SearchCol are dates in the format 20/01/2015 09:15:00, with no gaps or null values.

row_j is used to set End_row which is passed to Copy_to_b and propagates errors there.

Can anyone see where my problem is coming from?

Also, can you recommend a way to exit the loop when the desired value to match Date_end is met?

Thank you

Sub select_date_range(LastCol As Long, LastRow_date_new As Long, DateMax As Date, Date_end As Date)
Dim SearchCol As Integer
Dim row_i As Integer
Dim row_j As Integer
Dim Start_row As Integer
Dim End_row As Integer

With Worksheets("a")
    For SearchCol = 1 To LastCol Step 3
        LastRow_date_new = Application.CountA(.Range((.Cells(1, SearchCol)), (.Cells(65536, SearchCol))))
        For row_i = 2 To LastRow_date_new
            If Sheets("a").Cells(row_i, SearchCol).Value = DateMax Then Start_row = row_i
        Next row_i
        For row_j = LastRow_date_new To 2 Step -1
            If Sheets("a").Cells(row_j, SearchCol).Value = Date_end Then End_row = row_j
        Next row_j
        ''''''' use range col1, row i to col2, row j to copy into new sheet
        Call copy_to_b(Start_row, SearchCol, End_row)
    Next SearchCol
End With

End Sub

Upvotes: 2

Views: 602

Answers (2)

R3uK
R3uK

Reputation: 14537

With For row_j = LastRow_date_new To 2 Step -1, and after every For, you will have your variable equals to last value + step, this is how the For-loop exit itself.

So if you don't won't to have that variable to 1, you must set it to the right value after your For-loop, something like this :

    For row_i = 2 To LastRow_date_new
        If Sheets("a").Cells(row_i, SearchCol).Value = DateMax Then Start_row = row_i
    Next row_i
    row_i=2
    For row_j = LastRow_date_new To 2 Step -1
        If Sheets("a").Cells(row_j, SearchCol).Value = Date_end Then End_row = row_j
    Next row_j
    row_j=2

To exit the loop when your criteria is matched, you can use Exit For, some would say it's not elegant but it does work (and another way below code reviewed)

Sub select_date_range(LastCol As Long, LastRow_date_new As Long, DateMax As Date, Date_end As Date)
Dim SearchCol As Integer
Dim row_i As Integer
Dim row_j As Integer
Dim Start_row As Integer
Dim End_row As Integer

With Worksheets("a")
    For SearchCol = 1 To LastCol Step 3
        LastRow_date_new = Application.CountA(.Range((.Cells(1, SearchCol)), (.Cells(65536, SearchCol))))

        For row_i = 2 To LastRow_date_new
            If CDate(.Cells(row_i, SearchCol).Value) <> CDate(DateMax) Then
            Else
                Start_row = row_i
                Exit For
            End If
        Next row_i

        For row_j = LastRow_date_new To 2 Step -1
            If CDate(.Cells(row_j, SearchCol).Value) <> CDate(Date_end) Then
            Else
                End_row = row_j
                Exit For
            End If
        Next row_j


        ''''''' use range col1, row i to col2, row j to copy into new sheet
        Call copy_to_b(Start_row, SearchCol, End_row)
    Next SearchCol
End With

End Sub

To replace your For, you can use Do While ou Do Until :

row_j = LastRow_date_new
Do While row_j >= 2 And .Cells(row_j, SearchCol).Value <> Date_end
    row_j = row_j - 1
Loop
End_row = row_j

Upvotes: 2

cronos2546
cronos2546

Reputation: 1106

I believe that your problem may occur due to the absence of any values or only one value in a column that you review. Also, you may miss values if you have null values in this column, if this is the case, I will change my answer. You should change

With Worksheets("a")
    For SearchCol = 1 To LastCol Step 3
        LastRow_date_new = Application.CountA(.Range((.Cells(1, SearchCol)), (.Cells(65536, SearchCol))))
        For row_i = 2 To LastRow_date_new
            If Sheets("a").Cells(row_i, SearchCol).Value = DateMax Then Start_row = row_i
        Next row_i
        For row_j = LastRow_date_new To 2 Step -1
            If Sheets("a").Cells(row_j, SearchCol).Value = Date_end Then End_row = row_j
        Next row_j
        ''''''' use range col1, row i to col2, row j to copy into new sheet
        Call copy_to_b(Start_row, SearchCol, End_row)
    Next SearchCol
End With

To this

Dim xlws As Excel.Worksheet
Dim lngrow as Long
Set xlws = Thisworkbook.Sheets("a")
For SearchCol = 1 to LastCol Step 3
    While xlws.Range.Cells(lngrow,SearchCol).Value <> ""
          If xlws.Cells(lngrow,SearchCol).Value = DateMax Then
             Start_row = lngrow
          Elseif xlws.Cells(lngrow,SearchCol).Value = Date_end Then
             End_row = lngrow
          End If
          lngrow = lngrow + 1
     Loop
     Call copy_to_b(Start_row, SearchCol, End_row)
 Next SearchCol

Upvotes: 2

Related Questions