Anyone help to improve my code?

[Andrew]

Hi!

I've written a function to help with sorting items in a combo box in a
userform in Excel.

The code works fine, but I have a suspicion that there must be a more
elegant way of doing this - in particular I don't like using the
statement "Goto CheckLetter" which seems really clunky.

As you can see, the code takes two inputs - the combo box to receive
the new item (cbo) and the item to be analyzed and positioned
(NewItem). I've also included a sample of code that calls it, for
reference.

I'd appreciate any thoughts or comments!

Andrew
---------------------------
'Calling procedure
Private Sub UserForm_Initialize()
Dim rngNames As Range

' <----Irrelevant code cut from here --------->

'Create a loop that will add every name it finds in a range of cells
'into combo box cboStaff
Do Until rngNames.Value = ""
'Add the item, using the GetComboIndex function to determine the
'placement of the new item
cboStaff.AddItem rngNames.Value, GetComboIndex(cboStaff,
rngNames.Value)
Set rngNames = rngNames.Offset(1) 'Move to next cell in range
Loop

End Sub
-------------------------------------------
Function GetComboIndex(cbo As ComboBox, NewItem As String)
Dim i As Integer 'To loop with
Dim intLetterPos As Integer 'The letter position to be analyzed

intLetterPos = 1 'Start with the first letter

'First check whether the current item in the combo box
'has at each letter position a letter after the equivalent letter in
'the new word to be added

For i = 0 To cbo.ListCount - 1 'loop through for as many items as we
have in list
cbo.ListIndex = i

CheckLetter:

'If the letter in the current word in the combo box
'is AFTER the equivalent letter in the new word...

If Asc(StrConv(Mid(cbo.Text, intLetterPos, 1), vbUpperCase)) > _
Asc(StrConv(Mid(NewItem, intLetterPos, 1), vbUpperCase))
Then

'...we've found the position for the new word, so return that.
GetComboIndex = i
Exit Function

'If not, check if this is the SAME letter
ElseIf Asc(StrConv(Mid(cbo.Text, intLetterPos, 1), vbUpperCase)) =
_
Asc(StrConv(Mid(NewItem, intLetterPos, 1), vbUpperCase)) Then

'If so, go to the next letter...
intLetterPos = intLetterPos + 1
'... and check again
GoTo CheckLetter

'Otherwise we've not found the right index position yet, so loop to
'the next item in the combo box
End If
Next i

'If we've come out the end, it means that the new item is
'after the last of the items in the combo box, so send out an index
'value to reflect this.
GetComboIndex = i

End Function

 

[Guest]

Instead of doing a letter for letter compare,

compare the new name to the name in the listbox using strcomp which has an
argument to do a case insensitive compare

Function GetComboIndex(cbo As ComboBox, NewItem As String)
Dim i As Integer 'To loop with

i = 0
do while strcomp(newItem, cbo.List(i), vbTextCompare) <= 0
i = i + 1
if i >= cbo.Listcount then exit do
Loop

GetComboIndex = i

End Function

 

[Andrew]

Hmmm.... Thanks for that. I knew there was a better way!!

As a matter of interest, how exactly does strcomp work? I can see from
the help, for example, that "string1 is less than string2" will return
a value of -1, but how does the string become a value? On what basis
does this get judged? Is it based on the ASCII value of each letter in
the string (thus internally doing what I was trying to achieve)?

Thanks again for your help.

Andrew

 

[Guest]

Yes, I would assume internally it does what you were trying to do.

Just like the Sort on the menu must do that to sort cells of text.