Help with code! Newbie - Find Text - InsertBefore - Infinite loop problem

[jj2396]

Hi All,
New to VBA and really need help with this code. Any help would be much appreciated! :hi:

What I need the code to do is go find every instance of State & Zip code string within the body of a word document and insert a comma before the State. basically, this is part of a larger code which formats names and address.

I found snippets of codes which I modified the best I could but the code goes into an infinite loop when I run it and it inserts an infinite number of commas. the code eventually crashes and I have to close Word to recover from the crash.

This only happends when I use the "InsertBefore" method to insert the comma before the State. If I use the "InsertAfter" method, the code works great (no loop), but the comma is inserted after the state/zip code string which isn't what I want.

I haven't been able to solve this problem on my own.

In case anyone is wondering why the entire State/zip code string is included and not just the State, it's because a street could be called Victoria Street for instance or the name of a person could be Victoria and it would insert a comma before the street or the name of a person which isn't needed. Hence the search for the entire state/zipcode string before inserting the comma.

Here is the code



Sub Insert_comma()

Dim commaText As String
commaText = ","
Dim rTextStateZip As Range
Dim vFindStateZip(8) As String
Dim z As Long

' variable arrays for the different states with zipcodes as wildcards

vFindStateZip(0) = (" South Australia " & "???? ")
vFindStateZip(1) = (" Western Australia " & "???? ")
vFindStateZip(2) = (" Northern Territory " & "???? ")
vFindStateZip(3) = (" Tasmania " & "???? ")
vFindStateZip(4) = (" Victoria " & "???? ")
vFindStateZip(5) = (" New South Wales " & "???? ")
vFindStateZip(6) = (" Queensland " & "???? ")
vFindStateZip(7) = (" Australian Capital Territory " & "???? ")


For z = 0 To UBound(vFindStateZip)
With Selection
.HomeKey wdStory
With .Find
.ClearFormatting
.Replacement.ClearFormatting
Do While .Execute(FindText:=vFindStateZip(z), _
MatchWildcards:=True, _
MatchWholeWord:=True, _
MatchCase:=False, _
Wrap:=wdFindStop, Forward:=True) = True
Set rTextStateZip = Selection.Range 'The found text
With rTextStateZip 'Do what you want with the found text
Select Case z
Case Is = 0 'South Australia
rTextStateZip.InsertBefore (commaText)
Case Is = 1 'Western Australia
rTextStateZip.InsertBefore (commaText)
Case Is = 2 'Northern Territory
rTextStateZip.InsertBefore (commaText)


Case Is = 3 'Tasmania
rTextStateZip.InsertBefore (commaText)

Case Is = 4 'Victoria
rTextStateZip.InsertBefore (commaText)

Case Is = 5 'New South Wales
rTextStateZip.InsertBefore (commaText)

Case Is = 6 'Queensland
rTextStateZip.InsertBefore (commaText)
Case Is = 7 'Australian Capital Territory
rTextStateZip.InsertBefore (commaText)
End Select
End With
Loop 'and look for the next match
End With
End With
Next z


End Sub

[gmaxey]

Couple of things.

1. You demensioned your array to hold 9 items but you only defined 8.
2. Use range rather that selection whenever possible.
3. Collapse the range after your actions.

Option Explicit
Sub Insert_comma()
Dim commaText As String
commaText = ","
Dim rTextStateZip As Range
Dim vFindStateZip(7) As String 'You had (8) here.
Dim z As Long
Dim oRng As Word.Range

vFindStateZip(0) = (" South Australia " & "???? ")
vFindStateZip(1) = (" Western Australia " & "???? ")
vFindStateZip(2) = (" Northern Territory " & "???? ")
vFindStateZip(3) = (" Tasmania " & "???? ")
vFindStateZip(4) = (" Victoria " & "???? ")
vFindStateZip(5) = (" New South Wales " & "???? ")
vFindStateZip(6) = (" Queensland " & "???? ")
vFindStateZip(7) = (" Australian Capital Territory " & "???? ")


For z = 0 To UBound(vFindStateZip)
Set oRng = ActiveDocument.Content
With oRng.Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = vFindStateZip(z)
.MatchWildcards = True
.MatchWholeWord = True
.MatchCase = False
.Wrap = wdFindStop
.Forward = True
While .Execute
If Not oRng.Characters.First.Previous = "," Then
oRng = "," & oRng
End If
'Collapse the range, otherwise you just keep finding the same instance.
oRng.Collapse wdCollapseEnd
Wend
End With
Next z


End Sub

[jj2396]

Hi Greg,

Thank you very much for your reply! Much appreciated!
Problem Solved!

Just one minor thing
'You had

Rng.Collapse wdCollapseEnd

'Should be
oRng.Collapse wdCollapseEnd

Works perfectly! you may want to edit your post so other can maybe use the code as is from your post without modifications.

Again, really appreciate your help! was trying to figure it out myself for hours but I couldn't

JJ

[macropod]

Here's a simpler and far more efficient method:
Sub Insert_comma()
Dim StrStates As String, i As Long
StrStates = "South Australia,Western Australia,Northern Territory,Tasmania," & _
"Victoria,New South Wales,Queensland,Australian Capital Territory"
For i = 0 To UBound(Split(StrStates, ","))
With ActiveDocument.Content.Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = " " & Split(StrStates, ",")(i) & " [0-9]{4}>"
.Replacement.Text = ",^&"
.MatchWildcards = True
.Wrap = wdFindContinue
.Forward = True
.Execute Replace:=wdReplaceAll
End With
Next i
End Sub

[gmaxey]

Paul,

While not as simple, this might be more efficient as it won't result in double commas if one already exists:

Sub Insert_comma()
Dim StrStates As String, i As Long
StrStates = "South Australia,Western Australia,Northern Territory,Tasmania," & _
"Victoria,New South Wales,Queensland,Australian Capital Territory"
For i = 0 To UBound(Split(StrStates, ","))
With ActiveDocument.Content.Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = "([!,])" & "( " & Split(StrStates, ",")(i) & " [0-9]{4}>)"
.Replacement.Text = "\1,\2"
.MatchWildcards = True
.Wrap = wdFindContinue
.Forward = True
.Execute Replace:=wdReplaceAll
End With
Next i
End Sub


P.S. Where is the line between more efficient and far more efficient ;-)

[macropod]

Paul,

While not as simple, this might be more efficient as it won't result in double commas if one already exists:
That is a useful extra check as it makes the macro more effective. I don't think it affects efficiency one way or the other, though.

P.S. Where is the line between more efficient and far more efficient ;-)
Where I draw it ... In this case, a straightforward Find/Replace is 'far more efficient' than a loop that processes each item iteratively. Just time it on a document with a few thousand of each!

[gmaxey]

Paul,

Don't take offense. If I wrote anything that implied that I did, it was not intentional.

My initial objective in posting was not to provide the most efficient code, rather only provide something, based on the original that worked. Since I rarely use the selection object and can barely understand the extended .Execute construction, I modified those parts. I wanted to point out that the continous loops were due to a) an extra empty member in the array and b) not collapsing the range.

Your method is certainly more efficient in general. However, if it results in two commas where only one is desired then it becomes ineffective and inefficient. Wouldn't you agree?

[jj2396]

Hi Greg & Paul, Thanks for all your help guys, much appreciated! I'll be trying some of these new codes over the weekend!
One other point to make and issue I'm having is as follows.

Because this code is part of a larger code to format name & addresses,
it's important to search for the entire State & zipcode string before inserting the comma, not just the state, otherwise someone's name could be Victoria or the name of a business could be let's say South Australia so and so.
searching for the State name alone would mean that a comma would be inserted after someone's name etc.
I mentioned that in my original post

For instance, here is one of the string to be found before inserting a comma

(" South Australia " & "???? ")

The issue I'd like to resolve is, I want to make sure that the wildcards for the zipcode are numbers not letters before inserting a comma. That would make the code pretty much fail safe in terms of inserting a comma where i'ts not suppose to be.

In short, I'm searching for the State & a 4 number zipcode (0-9) followed by a space as per below.

(" South Australia " & "???? ")

I don't know how to specify that the zipcode wildcards should be numbers only and not letters so for now i'm using "???? ".

I tried "#### " as wildcards in an earlier version of this code and it didn't work.

Any help would be much appreciated!


Thanks again guys for all your help!

jj

[macropod]

Hi jj,

The code Greg and I have posted does check for a postcode, via the " [0-9]{4}>)" part of the Find expression, which looks for a 4-digit number. Greg's final variant also checks for the presence of a comma before the State name, so that an unnecessary extra one doesn't get added. FWIW, your '????' code could make false matches with 4-character words after the State name ...

[jj2396]

Hi Paul,

OK I see that now! Fantastic!

Thanks again for all your help! Will be trying this over the weekend.

Again, really grateful!

JJ

:)