Home > Software design >  VBA For loop by rows and columns
VBA For loop by rows and columns

Time:12-04

I have a bit of a complicated request (might be simple for some of you).

I have this small dataset where in which the top rows represent a Userform Combobox. enter image description here

I wanted to loop through each row to check if there is a match between what is currently picked in the userform and what is in this dataset (to see if this set of selections has already been picked).

If it's been picked then I wanted a MsgBox & Exit sub and if not then I want it to keep going through the loop till it reaches an empty cell.

This is what I attempted:

Sub check_procedures()
  Dim ws As Worksheet, rngF As Range
  
  Set ws = Worksheets("do not open!")
  Set rngF = ws.Range("F2")
   
   Do Until rngF.Value = ""
         If UI.ComboBoxSource = rngF.Value Then
         MsgBox "This procedure already exhists. Please click on Update Summary. "
         Exit Sub
         
         
         Else
         End If
            
            
         Set rngF = rngF.Offset(1)
         
         
    
    Loop
    
End Sub

It works but only for the first column (ComboBoxSource) but I don't know how to continue the loop trough the remaining columns, and I want the message MsgBox & Exit sub to appear only if all 5 conditions are met & not only one.

This is what I attempted to do:

  Sub check_procedures()
  Dim ws As Worksheet, rngF As Range, rngG As Range, rngH As Range, rngI As Range, rngJ As Range, Form_Date As Date
  
  Set ws = Worksheets("do not open!")
  Set rngF = ws.Range("F1")
  Set rngG = ws.Range("G1")
  Set rngH = ws.Range("H1")
  Set rngI = ws.Range("I1")
  Set rngJ = ws.Range("J1")
  Form_Date = UI.txtDay.Value & "/" & UI.txtMonth.Value & "/" & UI.txtYear
   
   
   Do Until rngF.Value = ""
         If UI.ComboBoxSource = rngF.Value Then
            If UI.ComboBoxLayer1 = rngG.Value Then
                If UI.ComboBoxLayer2 = rngH.Value Then
                    If UI.ComboBoxGrain = rngI.Value Then
                        If Form_Date = rngJ.Value Then
                        MsgBox "This procedure already exhists. Please click on Update Summary. "
                        Exit Sub
                        
                        Else
                        
                        End If
                    
                    Else
                    End If
                
                Else
                End If
            
            Else
            End If
            
         
         
         
         
         Else
         End If
            
            
         Set rngF = rngF.Offset(1)
         Set rngG = rngG.Offset(1)
         Set rngH = rngH.Offset(1)
         Set rngI = rngI.Offset(1)
         Set rngJ = rngJ.Offset(1)
         
         
    
    Loop
    
End Sub

It works but I believe it can be much better code and less repetitive.

I hope this explanation was clear.

Thanks in advance.

CodePudding user response:

The line Set rngF = rngF.Offset(1) moves rngF down by one each time, hence only going down the one column (there's nothing in this code to check another column)

EDIT: the only real variable here is what row you're checking, so there's no sense storing rngF, rngG etc as separate variables and moving them all separately. In the countif method of course that doesn't make any difference anyway.

Loop:

(Similar to your attempt, but you can use a single If statement and just one row variable)

Sub check_procedures()
    Dim ws As Worksheet, row as Long, Form_Date as Date
    Form_Date = CDate(UI.txtDay.Value & "/" & UI.txtMonth.Value & "/" & UI.txtYear)
    Set ws = Worksheets("do not open!")
    With ws
    Do Until .Range("F" & row) = ""
        If UI.ComboBoxSource = .Range("F" & row).Value _
        And UI.ComboBoxLayer1 = .Range("G" & row).Value _
        And UI.ComboBoxLayer2 = .Range("H" & row).Value _
        And UI.ComboBoxGrain = .Range("I" & row).Value _
        And Form_Date = .Range("J" & row).Value _
        Then
            MsgBox "This procedure already exists. Please click on Update Summary. "
            Exit Sub
        End If
        row = row   1
    Loop
    End With
End Sub

Single-line fix

(a little more difficult to write with multiple conditions, but runs quicker and simpler to look at)

Sub Check_procedures()
Dim ws As Worksheet, Form_Date as Date
Set ws = Worksheets("do not open!")
Form_Date = CDate(UI.txtDay.Value & "/" & UI.txtMonth.Value & "/" & UI.txtYear)
With ws
If WorksheetFunction.CountIfs( _
    .Range("F:F"), UI.ComboBoxSource, _
    .Range("G:G"), UI.ComboBoxLayer1, _
    .Range("H:H"), UI.ComboBoxLayer2, _
    .Range("I:I"), UI.ComboBoxGrain, _
    .Range("J:J"), Form_Date _
    ) > 0 Then
    MsgBox "This procedure already exists. Please click on Update Summary."
End If
End With
End Sub
  • Related