PDA

View Full Version : Solved: Help with SIMPLE string-matching loop



atarigen
01-30-2006, 09:58 PM
Hello everyone! This is my first post here and, as will soon be painfully obvious, I am a total newbie on VBA. I am in awe of you all, and in particular of the moderators of this forum. I can only begin to imagine how tedious and time consuming it must be to go through other people's codes, and yet everybody seems to get help here. I hope to one day know enough to contribute my own bit, but please bear with me for the time being...

Okay, so here is my problem. I've been trying to write a very simple code to help me classify inventory entries. One the one hand I have a long list of item descriptions that do not follow any uniform standard. On the other I have the names of most of the relevant designers and manufacturers. The macro I wrote was supposed to compare each item description with the entries in the designer list, and return in an adjascent column any matches or partial matches. Unfortunately it doesn't run at all and I haven't been able to find what is wrong with it.

Here is what I have so far:



Function FindStrMatch(strX As String) As String

Dim wsSource As Worksheet, i As Long, strSource As Range

Set wsSource = Sheets("Designers")

With wsSource
For i = 1 To Range("A65536").End(xlUp).Row
Set strSource = Range(A, i)
If InStr(strSource.Text, strX) > 0 Then
FindStrMatch = strSource.Text
Exit Function
End If
Next i
End With

End Function

Sub GetMatches()

Dim wsSource As Worksheet, wsTarget As Worksheet, strX As String
Dim cl As Range, strMatch As String

Set wsSource = Sheets("Designers")
Set wsTarget = Sheets("ItemDesc")

For Each cl In wsTarget.Range("A:A")
If cl <> "" Then
Set cl.Text = strX
strMatch = FindStrMatch(strX)
wsTarget.Range(cl.Address).Offset(0, 1).Value = strMatch
End If
Next cl

End Sub




The only error message I get is "Object required" - ??! Any ideas on what may be missing? I would also appreciate any suggestions on how to improve the code, especially regarding contingency management (i.e. errors, null results etc).

Thanks in advance for your time!

XLGibbs
01-30-2006, 10:04 PM
Hi! Welcome to VBAX!

Okay, here is a quick rewrite to correct for syntax...untested


Function FindStrMatch(byVal strX As String) As String
Dim wsSource As Worksheet, i As Long, strSource As Range

Set wsSource = Sheets("Designers")

With wsSource
For i = 1 To Range(.rows.count).End(xlUp).Row
strSource = Range("A" & i) 'can also be strSource = cells(i,1)

If InStr(strSource, strX) Then

FindStrMatch = strSource

Exit Function

End If
Next i
End With

End Function

Sub GetMatches()

Dim wsSource As Worksheet, wsTarget As Worksheet, strX As String
Dim cl As Range, strMatch As String

Set wsSource = Sheets("Designers")
Set wsTarget = Sheets("ItemDesc")
With wsTarget
For Each cl In .Range("A1:A" & .rows.count).End(xlup)
If cl <> "" Then
strx = cl
strMatch = FindStrMatch(strX)

cl.Offset(0, 1) = strMatch ' the for... each sets cl as a range,

End If
Next cl

End Sub


Give that a whirl and shout back.

Untested, very quick rewrite...but should be okay..

XLGibbs
01-30-2006, 10:13 PM
You may also try this quick rewrite...you don't need both loops and an external function...

Purely syntax based, untested

Sub GetMatches()

Dim wsSource As Worksheet, wsTarget As Worksheet, strX As String
Dim cl As Range, strMatch As String, rngLook As Range, rngFind As Range, rngMatch As Range

Application.ScreenUpdating = False
Application.EnableEvents = False

Set wsSource = Sheets("Designers")
Set wsTarget = Sheets("ItemDesc")
'set the ranges to look for, and the look for what?
Set rngLook = wsSource.Range("A1:A" & wsSource.Rows.Count).End(xlUp)
Set rngFind = wsTarget.Range("A1:A" & wsTarget.Rows.Count).End(xlUp)

For Each cl In rngFind

'set a range reference to a found cell in rngFind with the contents of cl within rngLook
Set rngMatch = rngLook.Find(cl)
'if not found then line it up 1 column over..
If Not rngMatch Is Nothing Then
cl.Offset(, 1) = rngMatch
End If
next cl
Application.ScreenUpdating = True
Application.EnableEvents = True

End Sub


Hope it helps!

malik641
01-30-2006, 10:20 PM
I see Gibbs beat me to the punch....but I re-wrote your code stating some pointers you may want to look at. Also, I did test this code...and I believe its what you wanted. Looking at Gibbs code I believe it will work too :yes

Option Explicit

Function FindStrMatch(strX As String) As String

Dim wsSource As Worksheet, i As Long, strSource As Range

Set wsSource = Sheets("Designers")

With wsSource
'Try to use Rows.Count when findind the last row. It's better practice
For i = 1 To Range("A" & Rows.Count).End(xlUp).Row
'Ranges such as column A must be defined with quotes: "A"
'And when you are combining "A" with i, you need to use the
'ampersand symbol "&" like so:
Set strSource = Range("A" & i)
If InStr(strSource.Text, strX) > 0 Then
FindStrMatch = strSource.Text
Exit Function
End If
Next i
End With

'Be sure to set "Set" variables to nothing. Good practice.
Set wsSource = Nothing
End Function

Sub GetMatches()

Dim wsSource As Worksheet, wsTarget As Worksheet, strX As String
Dim cl As Range, strMatch As String, iLastRow As Long

Set wsSource = Sheets("Designers")
Set wsTarget = Sheets("ItemDesc")
iLastRow = wsTarget.Range("A" & Rows.Count).End(xlUp).Row

'Old For statement would have went through each cell in
'column A, which could be very slow. New For statement only
'moves through the range used.
For Each cl In wsTarget.Range("A1", "A" & iLastRow)
If cl <> "" Then
'This statement was backwards and you don't have to "Set"
'String variables. This is where you had a "Object Required" error.
strX = cl.Text
strMatch = FindStrMatch(strX)
wsTarget.Range(cl.Address).Offset(0, 1).Value = strMatch
End If
Next cl

Set wsSource = Nothing
Set wsTarget = Nothing
End Sub
Hope this helps you understand a little better :thumb

malik641
01-30-2006, 10:24 PM
Also, I edited your post to include VBA tags :thumb I think you made a tiny error by putting the closing tag with an additional space "[VBA/ ]"....no biggie :)

XLGibbs
01-30-2006, 10:25 PM
Good advice indeed. Tis my enemy leaving all those rogue objects floating around. http://vbaexpress.com/forum/images/smilies/banghead.gif

'Be sure to set "Set" variables to nothing. Good practice.

malik641
01-30-2006, 10:30 PM
Good advice indeed. Tis my enemy leaving all those rogue objects floating around. http://vbaexpress.com/forum/images/smilies/banghead.gif

'Be sure to set "Set" variables to nothing. Good practice.

As mine too....I'm still trying to get into that good habit. I figure if I tell everyone it'll be second nature http://vbaexpress.com/forum/images/smilies/001.gif

atarigen
01-31-2006, 01:04 AM
Are you guys for real??! I cannot BELIEVE how fast you were! Thank you so much! I also really appreciate the tips and rewrites... I got a long way to go in my quest for taming this beast!

Gibbs - Your first revision wouldn't run. VBA generated an error message box but it was blank?? As for the second one, it runs but doesn't seem to really do anything... I know you were just doing the syntax so the error must be in my conceptualization.

Malik - Your code ran perfectly smoothly. And thanks for the great notes! The only thing is that the output isn't what I had intended... I don't understand why it is doing this.

Perhaps you guys could tell me better what is going on if you have a look at the whole file? (sorry for not including it earlier)

Also, I don't think I was particularly clear about a very important detail... I would like the output to be the Designer name only... What Malik's code is returning is the full Item description, which is also what Gibb's code (the second one) seems to be doing? The string with the Designer name should be included in the Item Description - the two strings will never be a perfect match.

I hope I am not overstaying my welcome. Thanks again for the great help!

xo
Ana

Bob Phillips
01-31-2006, 01:07 AM
Good advice indeed. Tis my enemy leaving all those rogue objects floating around. http://vbaexpress.com/forum/images/smilies/banghead.gif

'Be sure to set "Set" variables to nothing. Good practice.


Not necessarily - see http://tinyurl.com/a56pd.

The statement that really hit home to me was when Matthew said, '... I've also noticed that many people who are religious about the Set = Nothing
do not take the same approach to strings and array types. They seem to be perfectly happy with letting VB clean these types, but still religiously maintain that it can't handle object types.'

TonyJollans
01-31-2006, 05:30 AM
Whoa!

I haven't read all of it, but that thread is about VB and .NET. VBA is not necessarily the same. VBA 6.3 (in Office 2003) seems to have improved housekeeping but you should not rely on it to do what you want. One must trust one's environment to do a certain amount of clean up, although historically that has sometimes proved to be less than reliable, but that does not mean that one should not also do one's own housekeeping as well. It is good practice to explicitly set object variables to nothing.

Object variables are different from non-object variables in that they may point to, and may have been directly responsible for creating, objects which have an independent existence outside the code environment which will be cleaned up. Unless one has a very clear understanding (and most people don't) of what goes out of scope and when, one should, at the very least, play it safe. This is particularly important in VBA where the Parent Application continues after code has ended, which is rather different from a stand-alone VB (or .NET) Application.

Just because some people code some things badly (jumping out of With blocks was one example given) is not a reason for other people to code badly in other ways. And cleaning up strings and/or arrays after use may sometimes be good practice but they are not the same as objects and the same regimen is not necessarily appropriate.

malik641
01-31-2006, 09:34 AM
Okay, I believe I have what you're looking for http://vbaexpress.com/forum/images/smilies/023.gif

We missed some dots within your With statement in the Function...no biggie. And we had the syntax backwards for the Instr function:

The syntax for the InStr function is:

InStr( [start], string_being_searched, string2, [compare] )

start is optional. It is the starting position for the search. If this parameter is omitted, the search will begin at position 1.

string_being_searched is the string that will be searched.

string2 is the string to search for.

compare is optional. This is the type of comparison to perform. The valid choices are: vbBinaryCompare, vbTextCompare, and vbDatabaseCompare.

And we switched string_being_searched with string2...oops http://vbaexpress.com/forum/images/smilies/doh.gif



But in anycase, here ya go:


Option Explicit

Function FindStrMatch(strX As String) As String

Dim wsSource As Worksheet, i As Long, strSource As Range
Dim cellText As String

Set wsSource = Sheets("Designers")

With wsSource
For i = 1 To .Range("A" & Rows.Count).End(xlUp).Row
cellText = LCase(.Range("A" & i))
strX = LCase(strX)
If InStr(strX, cellText) > 0 Then
FindStrMatch = .Range("A" & i).Text
Set wsSource = Nothing
Exit Function
End If
Next i
End With

Set wsSource = Nothing
End Function

Sub GetMatches()

Dim wsSource As Worksheet, wsTarget As Worksheet, strX As String
Dim cl As Range, strMatch As String, iLastRow As Long

Set wsSource = Sheets("Designers")
Set wsTarget = Sheets("ItemDesc")
iLastRow = wsTarget.Range("A" & Rows.Count).End(xlUp).Row

For Each cl In wsTarget.Range("A1", "A" & iLastRow)
If cl <> "" Then
strX = cl.Text
strMatch = FindStrMatch(strX)
wsTarget.Range(cl.Address).Offset(0, 1).Value = strMatch
End If
Next cl

Set wsSource = Nothing
Set wsTarget = Nothing
End Sub



As for Bob and Tony's posts...Nice info...but I'm torn on what to think now. I do believe that VBA and VB & .NET are different without a doubt. And I am reading the eBook on .NET and it does mention that it takes care of setting variables to nothing once it leaves the scope. But....how much run-time are you actually killing by "Duplicating existing code"??? I say another test is in order! Where's MWE??? http://vbaexpress.com/forum/images/smilies/001.gif

Bob Phillips
01-31-2006, 10:54 AM
I haven't read all of it, but that thread is about VB and .NET. VBA is not necessarily the same. VBA 6.3 (in Office 2003) seems to have improved housekeeping but you should not rely on it to do what you want. One must trust one's environment to do a certain amount of clean up, although historically that has sometimes proved to be less than reliable, but that does not mean that one should not also do one's own housekeeping as well. It is good practice to explicitly set object variables to nothing.

The article does not differentiate as clearly (or even at all as I recall) between VB and VBA as you do, and I can't say for sure as I don't have access to the source, but I bet a lot of VB and VBA is the same code. Can you be sure that it is VBA that has improved housekeeping (and can you actually demonstrate that), and it is not in shared DLLs?


Object variables are different from non-object variables in that they may point to, and may have been directly responsible for creating, objects which have an independent existence outside the code environment which will be cleaned up. Unless one has a very clear understanding (and most people don't) of what goes out of scope and when, one should, at the very least, play it safe. This is particularly important in VBA where the Parent Application continues after code has ended, which is rather different from a stand-alone VB (or .NET) Application.

I think you are thinking of examples to support your view here. Matthew is not differentiating between object variables and other variables, that is the whole point of his argument, saying that we trust the system to tidy simple variable, why not object variables. He should know, he is a primary developer in the MS VB team (or was). And it is not different, an object variable declared in a procedure goes out of scope at the end of that procedure, and there is still code executing, so different containers, similar situations.


Just because some people code some things badly (jumping out of With blocks was one example given) is not a reason for other people to code badly in other ways. And cleaning up strings and/or arrays after use may sometimes be good practice but they are not the same as objects and the same regimen is not necessarily appropriate.

Again, I would state that the whole crux of the argument asks what is the justification for treating them differently. If you don't trust MS to tidy-up object variables, why do you trust them to tidy-up object variables? And what about implicit objects such as


With Worksheets(1)
Msgbox .Range("A1")
MsgBox .Range("A2")
End With


how are you going to explicitly release those?

There is only one problem that I know of with objects remaining in memory, and that is in automation, when quitting doesn't sometimes quit. But my guess is that this is to do with the implementation of automation, and further, I wouldn't be surprised if the same problem manifests in automation in VB (hard to prove, as I have never been able to readily reproduce the problem).

I am not personally advocating not explicitly releasing objects, I try to do it as a matter of course myself, because it does no harm, and it is another example of defensive programming (although Matthew does show problems in doing so, I have never had those problems), but I cannot see bland, unsubstantiated statements going unchallenged. Be honest, when did you last have a problem not releasing objects in a non-automation situation, one that you could definitely demonstrate as such?

atarigen
01-31-2006, 12:14 PM
Oooohhhhhhhhhhh

:cloud9: :cloud9: :cloud9: :cloud9: :cloud9:


I've written a new code:

Function EternalGratitude(VBAx As Heaven, Malik As Messiah) As Hyperbole

Set Malik = BestFriend

Do
VBAx.Malik.SmotheringKisses
Loop Until Malik = CantBreathe

End Function

Surely in this case it is okay not to set variables to nothing? :devil:



P.S. Should I change this post's title to "Solved"? Or should it be left alone while you guys are still debating the merits of tidying up variables?

TonyJollans
01-31-2006, 01:36 PM
I'm not sure I want to get into a long argument, but here goes :)


The article does not differentiate as clearly (or even at all as I recall) between VB and VBA as you do, and I can't say for sure as I don't have access to the source, but I bet a lot of VB and VBA is the same code. Can you be sure that it is VBA that has improved housekeeping (and can you actually demonstrate that), and it is not in shared DLLs?
The article does not mention VBA (as far as I could see). And I maintain that running inside a parent application is different from running stand-alone. 'tis true, I can't immediately demonstrate anything but I don't have an environment where I can easily check memory usage and the efficacy (or otherwise) of housekeeping. I don't know where the (relevant) code is, and it may well be in a shared DLL. But when, or whether, and how that code is invoked from a calling environment is at least as important as the code itself.

My experience of many different environments over many years leads me not to trust system housekeeping to be 100% effective. If there is just one environment with one unusual set-up where it doesn't work then making everything you can explicit pays off. Maybe it all works properly now but (as noted) I can't prove or disprove that. Whilst I think that MS do a pretty good job on the whole, they are subject to the same commercial pressures as the rest of the world and bugs do escape; why take the chance?



I think you are thinking of examples to support your view here. Matthew is not differentiating between object variables and other variables, that is the whole point of his argument, saying that we trust the system to tidy simple variable, why not object variables. He should know, he is a primary developer in the MS VB team (or was). And it is not different, an object variable declared in a procedure goes out of scope at the end of that procedure, and there is still code executing, so different containers, similar situations.
Yes, of course I am thinking of examples to support my view :)

I don't know Matthew, or his r?le, but many's the time I've heard developers swear their code does this, that, or the other only to find out it doesn't always do so at all. He may be saying there is no difference between object and non-object variables but I tend to disagree; there are differences within those broad groupings but there are significant differences between them.

Some non-object variables are pointers, some not, but even the pointer types point to memory that is under VBA's control and I have no choice but to rely on the housekeeping - that doesn't mean I trust it. Object variables, on the other hand, point to objects outside VBA's control and I have no way of knowing what VBA may do - of course I have no way of knowing what VBA may do if I explicitly set one to Nothing either! What should I do? Flip a coin? I have to go on experience which leads me to believe objects should be explicitly cleared - over time that may change but it won't be for a while yet.

An exception to the above generalisation is Collections; these are built-in objects which VBA's housekeeping really ought to handle but I have certainly had problems with them not being properly tidied up and would always want to see them explicitly set to nothing.



Again, I would state that the whole crux of the argument asks what is the justification for treating them differently. If you don't trust MS to tidy-up object variables, why do you trust them to tidy-up object variables? And what about implicit objects such as


With Worksheets(1)
Msgbox .Range("A1")
MsgBox .Range("A2")
End With


how are you going to explicitly release those?
I don't entirely trust MS to tidy up non-object variables but, as said, I have no real choice. I do sometimes explicitly set Strings to vbNullString; arrays can be more complex and with UDTs you get into a whole new ball park.

I can't do anything about that code example. I consider the End With as explicit clearing of the pointer created by the With. I don't know whether or not any object variable is created for the Ranges, never mind destroyed, and I have to rely on VBA getting it right. It does not follow that I should rely on them getting it right in other situations where I do have a choice.



There is only one problem that I know of with objects remaining in memory, and that is in automation, when quitting doesn't sometimes quit. But my guess is that this is to do with the implementation of automation, and further, I wouldn't be surprised if the same problem manifests in automation in VB (hard to prove, as I have never been able to readily reproduce the problem).
So, there is a problem :) I wasn't making a distinction between object variables used in automation and any others. Nobody else was making that distinction either. Does housekeeping work with object variables or does it only work with some of them?



I am not personally advocating not explicitly releasing objects, I try to do it as a matter of course myself, because it does no harm, and it is another example of defensive programming (although Matthew does show problems in doing so, I have never had those problems), but I cannot see bland, unsubstantiated statements going unchallenged. Be honest, when did you last have a problem not releasing objects in a non-automation situation, one that you could definitely demonstrate as such?
The thought that explicitly releasing objects can cause problems bothers me a bit but I haven't come across this happening so I'll carry on doing it.

You are of course right to make us aware that long held assumptions do not necessarily remain valid through various releases of software. I suspect most people here routinely set objects to nothing but I would be interested to hear of anyone's experience which might suggest it not necessary in any particular (VBA) environment.

I've rambled on far too much but, now I'm in thinking mode, why are object variables not automatically cleared when the object is explicitly destroyed? Why is it that I must do, say,appExcel.Quit
Set appExcel = NothingThe housekeping that knows there is a pointer should clear the pointer rather than not release the object, surely. It's partly this kind of anomaly that makes me suspicious.

malik641
01-31-2006, 01:55 PM
Oooohhhhhhhhhhh

:cloud9: :cloud9: :cloud9: :cloud9: :cloud9:


I've written a new code:

Function EternalGratitude(VBAx As Heaven, Malik As Messiah) As Hyperbole

Set Malik = BestFriend

Do
VBAx.Malik.SmotheringKisses
Loop Until Malik = CantBreathe

End Function

Surely in this case it is okay not to set variables to nothing? :devil:



P.S. Should I change this post's title to &quot;Solved&quot;? Or should it be left alone while you guys are still debating the merits of tidying up variables?
:rotlaugh::giggle :cloud9:

Atarigen,
This is byfar the best compliment I've ever received :cloud9::cloud9::cloud9:. Thank you very much :rofl: :yes. I look forward to helping you much more!!! :) :yes

And I'm sure even if you marked it solved, they'll still go back and forth with it :yes. And if you don't want to and you forget...I'll probably do it for you :thumb



...who's jealous of the smothering kisses?? :whistle:

XLGibbs
01-31-2006, 02:20 PM
This prompts me to develop a new function

Private Sub Nausea (byVal Compliment as Boolean, strInput as String, x as Integer) as Boolean

If (Compliment = True and Len(strInput > 0) ) * x > 0 then
Nausea = True
MsgBox "I might hurl"
Else
Nausea = False
msgBox "Thank you"
End if
End


I am enjoying the lively debate that has ensued though :)

Bob Phillips
01-31-2006, 05:56 PM
This prompts me to develop a new function

Private Sub Nausea (byVal Compliment as Boolean, strInput as String, x as Integer) as Boolean

If (Compliment = True and Len(strInput > 0) ) * x > 0 then
Nausea = True
MsgBox "I might hurl"
Else
Nausea = False
msgBox "Thank you"
End if
End


I am enjoying the lively debate that has ensued though :)

How about

Function Finger()
MsgBox "Up"
End Function

Bob Phillips
01-31-2006, 05:58 PM
I'm not sure I want to get into a long argument, but here goes :)

Tony,

This doesn't seem the correct place to debate this, so I think I will start a thread over at Dennis' ExcelKB. It will take me a day or so to gather some points to make, and then we can air it in a more structured forum.

I hope that you will visit and contribute.

Bob

XLGibbs
01-31-2006, 07:04 PM
How about

Function Finger()
MsgBox "Up"
End Function

I was just kidding! but I am flattered that you think I am #1 :p

TonyJollans
02-01-2006, 12:15 AM
OK, Bob - I'll watch out for it. Some different people over there - it should be interesting.

malik641
02-01-2006, 05:24 AM
I think the REAL function would be...

Function Gibbs()
Msgbox "Is just jealous" & vbCrLf & "& I'm not in any way offended by Gibbs' new function because I know the truth "
End Function

:) :rofl: