tangerinev43
tangerinev43

Reputation: 41

VBA loop is not looping properly

When I go through the for loop, it starts off working, but as I go down the column the cells are not deleted nor highlighted in red.

For i = 2 To lngRow
    
    If Cells(i, 27).Value = "Completed" Or IsEmpty(Cells(i, 27).Value) = True Then
        'Do nothing
    Else
        'Go into the exception items'
        If Cells(i, 28).Value = "Contact Information Not Found" Then
            'Check Code: If identifiable (4 digits long + starts with a F), highlight
            If Len(Cells(i, 4).Value) = 4 And Left(Cells(i, 4), 1) = "F" Then
                Cells(i, 28).Interior.Color = RGB(255, 0, 0)
            Else
                'Otherwise, delete
                Rows(i).EntireRow.Delete
            End If
                
        ElseIf Cells(i, 28).Value = "Invalid Account Number." Then
        End If
    End If
        
Next i

Upvotes: 4

Views: 176

Answers (3)

VBasic2008
VBasic2008

Reputation: 54948

Color and Delete

  • If you would have added Sub and End Sub and defined lngRow, it would have been a minimal reproducible example.
  • I prefer more meaningful variable names like LastRow instead of the Hungarian Notation lngRow which you will nowadays rarely see, especially on this site.
  • When you evaluate two conditions in one row, both are being evaluated. Why evaluate the second when the first is already False?
  • Creating a procedure like builtRange and using it to 'build' ('combine') ranges will be more efficient than always accessing the worksheet for each range. Using it will reduce accessing the worksheet to one time only (per operation).
  • The last procedure is just a thought on how to test against a non-blank cell (instead of an non-empty cell). Among others, it concludes that it is maybe best done by using If Len(cel.value) > 0 Then. It creates a new workbook and does operations in it, so all other workbooks are safe.

The Code

Option Explicit

Sub colorOrDelete()
    
    Dim LastRow As Long: LastRow = 20
    
    Dim drg As Range ' Delete Range
    Dim crg As Range ' Color Range
    Dim i As Long ' Worksheet Rows Counter
    
    For i = 2 To LastRow
        If Not IsEmpty(Cells(i, 27)) Then
            If Cells(i, 27).Value <> "Completed" Then
                If Cells(i, 28).Value = "Contact Information Not Found" Then
                    If Len(Cells(i, 4).Value) = 4 Then
                        If Left(Cells(i, 4), 1) = "F" Then
                            buildRange crg, Cells(i, 28)
                        Else
                            buildRange drg, Rows(i)
                        End If
                    End If
                End If
            ' An Idea
            'ElseIf Cells(i, 28).Value = "Invalid Account Number." Then
            'Else
            End If
        End If
    Next i
    
    Dim hasChanged As Boolean
    
    Application.ScreenUpdating = False
    
    ' You wanna color the cells before deleting the rows.
    If Not crg Is Nothing Then
        hasChanged = True
        crg.Interior.Color = RGB(255, 0, 0)
    End If
    
    If Not drg Is Nothing Then
        If Not hasChanged Then
            hasChanged = True
        End If
        drg.Delete
    End If

    Application.ScreenUpdating = True
    
    If hasChanged Then
        MsgBox "Operation finished.", vbInformation, "Success"
    Else
        MsgBox "Done nothing.", vbExclamation, "No Change"
    End If
    
End Sub

Sub buildRange( _
        ByRef builtRange As Range, _
        AddRange As Range)
    If builtRange Is Nothing Then
        Set builtRange = AddRange
    Else
        Set builtRange = Union(builtRange, AddRange)
    End If
End Sub

Some Thoughts

Sub buildRangeParanoia( _
        ByRef builtRange As Range, _
        AddRange As Range)
    If Not AddRange Is Nothing Then
        If builtRange Is Nothing Then
            Set builtRange = AddRange
        Else
            If AddRange.Worksheet Is builtRange.Worksheet Then
                Set builtRange = Union(builtRange, AddRange)
            End If
        End If
    End If
End Sub

Sub EmptyVsBlank()
    
    Dim wb As Workbook: Set wb = Workbooks.Add
    
    With wb.Worksheets(1)
        
        Dim cel1 As Range: Set cel1 = .Range("A1"): cel1.Value = Empty
        Dim cel2 As Range: Set cel2 = .Range("A2"): cel2.Value = "="""""
        Dim cel3 As Range: Set cel3 = .Range("A3"): cel3.Value = "'"
        
        Debug.Print IsEmpty(cel1), IsEmpty(cel1.Value), Len(cel1.Value)
        Debug.Print IsEmpty(cel2), IsEmpty(cel2.Value), Len(cel2.Value)
        Debug.Print IsEmpty(cel3), IsEmpty(cel3.Value), Len(cel3.Value)
        
        Dim rg As Range: Set rg = .Range("A1:A3")
        Debug.Print Application.CountA(rg) ' A2, A3
        Debug.Print Application.CountBlank(rg) ' A1, A2, A3
        ' The Shock
        Debug.Print Application.CountIf(rg, Empty) ' None
        Debug.Print Application.WorksheetFunction.CountIf(rg, Empty) ' None
   
        .Parent.Saved = True ' To close without prompt.
        '.Parent.Close SaveChanges:=False
   
   End With
    
    ' Result:
    ' True          True           0
    ' False         False          0
    ' False         False          0
    ' 2
    ' 3
    ' 0
    ' 0

End Sub

Upvotes: 0

DerinNehir
DerinNehir

Reputation: 56

Loop backwards:

For i = lngRow To 2 Step-1   'here

    If Cells(i, 27).Value <> "Completed" And IsEmpty(Cells(i, 27).Value) = False Then
         'Go into the exception items'
        If Cells(i, 28).Value = "Contact Information Not Found" Then
            'Check Code: If identifiable (4 digits long + starts with a F), highlight
            If Len(Cells(i, 4).Value) = 4 And Left(Cells(i, 4), 1) = "F" Then
                Cells(i, 28).Interior.Color = RGB(255, 0, 0)
            Else
            'Otherwise, delete
                Rows(i).EntireRow.Delete
            End If
            
        ElseIf Cells(i, 28).Value = "Invalid Account Number." Then
        End If
    End If
    
Next i

Upvotes: 2

NickSlash
NickSlash

Reputation: 5100

Try adding the line below your delete statement. (I'm on my phone so can't edit very well)

' otherwise, delete
Rows(i).EntireRow.Delete
i=i-1 'adjust the index

You may need to adjust lngRow each time too so you don't try to process empty rows at the bottom of your data

Upvotes: 1

Related Questions