CODEWITHSUNDEEP
vbaexcel
vagon
vagon

Reputation: 77

VBA: Ignoring Conditional in For-Each Loop

Problem Statement

I have a couple of dependent combo boxes for some countries and states of those countries. I am using VBA to populate unique values in the first combo box and then dynamically populate unique values in the second combo box. The code seems to be ignoring the conditional in the initial pass.

For example the code works for the first country:

enter image description here

But following countries incorrectly retain the first State value:

enter image description here

Data

This is the data set, with the Names "Country" and "State". These Names correspond dynamically to the range below each heading:

enter image description here

Name references use formulas in this format:

=OFFSET(Sheet1!$A$2,0,0,COUNTA(Sheet1!$A:$A),1)

Combo boxes are ActiveX objects with the names "countries" and "states" respectively.

Code

Code snippet: 

Private Sub Worksheet_Activate()
'Populate combo box with unique countries.
  Dim arr() As String
  Dim tmp As String
  Dim rng As Range
  Dim ws As Worksheet
  Set ws = Worksheets("Sheet1")
  Me.countries.Clear
  For Each rng In ws.Range("Country")
    If (rng <> "") And (InStr(tmp, rng) = 0) Then
      tmp = tmp & rng & "|"
    End If
  Next rng

  If Len(tmp) > 0 Then tmp = Left(tmp, Len(tmp) - 1)

  arr = Split(tmp, "|")

  Me.countries.List = arr

End Sub

Private Sub countries_lostfocus()
  'Populate dependent combo box with unique states
  'according to selection in countries combo box.
  Dim rng As Range
  Dim ws As Worksheet
  Dim str As String
  Set ws = Worksheets("Sheet1")
  str = countries.Value
  Me.states.Clear
  On Error Resume Next
  For Each rng In ws.Range("State")
    If ((rng.Offset(, -1).Value) = str) And (IsNotInArray(rng.Value, Me.states.List)) Then
      Me.states.AddItem rng.Value
    End If
  Next rng
End Sub

Function IsNotInArray(stringToBeFound As String, arr As Variant) As Boolean
  IsNotInArray = IsError(Application.Match(stringToBeFound, arr, 0))
End Function

Miscellaneous

The NSW state value will be stored in the combo box for all following countries that are added.

Using MsgBox to debug inside the loop as such:

  For Each rng In ws.Range("State")
    If ((rng.Offset(, -1).Value) = str) And (IsNotInArray(rng.Value, Me.states.List)) Then
      MsgBox ("Country: " & str & "; check: " & rng.Offset(, -1).Value)
      Me.states.AddItem rng.Value
    End If
  Next rng

Seems to show that the first portion of the conditional is failing to operate as expected when selecting a country other than Australia:

enter image description here

Upvotes: 3

Views: 185

Answers (2)

PatricK
PatricK

Reputation: 6433

You could have use the same approach as in Country. And why don't you use the countries_Change event?

Option Explicit

Private Sub countries_Change()
    Dim sCountry As String
    Dim sList As String
    Dim rng As Range
    sCountry = Me.countries.Value
    Me.states.Clear
    With ThisWorkbook.Names("State")
        For Each rng In .RefersToRange
            If Not IsEmpty(rng) Then
                If rng.Offset(0, -1).Value = sCountry Then
                    If InStr(1, sList, rng.Value, vbTextCompare) = 0 Then
                        If Len(sList) > 0 Then sList = sList & "|"
                        sList = sList & rng.Value
                    End If
                End If
            End If
        Next
    End With
    Me.states.List = Split(sList, "|")
End Sub

Private Sub Worksheet_Activate()
    Dim sList As String
    Dim rng As Range
    With ThisWorkbook.Names("Country")
        For Each rng In .RefersToRange
            If Not IsEmpty(rng) Then
                If InStr(1, sList, rng.Value, vbTextCompare) = 0 Then
                    If Len(sList) > 0 Then sList = sList & "|"
                    sList = sList & rng.Value
                End If
            End If
        Next
    End With
    Me.countries.List = Split(sList, "|")
    countries_Change ' <-- This is better User experience
End Sub

Upvotes: 1

YowE3K
YowE3K

Reputation: 23994

As much as I don't want to see NSW being left out of any lists, you can fix your problem by testing whether your arr variable is empty prior to trying to do a Match:

Function IsNotInArray(stringToBeFound As String, arr As Variant) As Boolean
    If UBound(Arr) = -1 Then
        IsNotInArray = True
    Else
        IsNotInArray = IsError(Application.Match(stringToBeFound, arr, 0))
    End If
End Function

If arr is passed to that function as the cleared list of a ComboBox, it will have a LBound of 0 and an UBound of -1, so the test on the UBound will prevent the Match from crashing.

Upvotes: 2

Related Questions