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
.
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