Code works but looking for help in streamlining

[lkpederson]

I have a macro that corrects many punctuational, etc. issues with documents. The only two lines which change are the ".Text =" and the ".Replacement.Text =" lines. I'd like assistance in making the code more efficient.

Example of some of the code:


' gets rid of tabs at end of line
With cRng.Find
.Forward = True
.Text = "([0-9A-z])(^9){1,}(^13){1,}"
.Replacement.Text = "\1\3"
.MatchCase = True
.MatchWildcards = True
.Wrap = wdFindContinue
.Execute Replace:=wdReplaceAll
End With

' adds two spaces after colons.
With cRng.Find
.Forward = True
.Text = "([:])( )([0-9A-z])"
.Replacement.Text = "\1 \3"
.MatchCase = True
.MatchWildcards = True
.Wrap = wdFindContinue
.Execute Replace:=wdReplaceAll
End With

' changes +- symbol to "plus or minus"
With cRng.Find
.Forward = True
.Text = Chr$(177)
.Replacement.Text = "plus or minus"
.MatchCase = True
.MatchWildcards = True
.Wrap = wdFindContinue
.Execute Replace:=wdReplaceAll
End With

TIA.

[macropod]

I'd consider implementing the lot as a loop but, even with it implemented linearly, you can omit:
.MatchCase = True
altogether and you can omit:
.MatchWildcards = True
.Wrap = wdFindContinue
after the first instance.

It appears some of your F/R expressions are unnecessarily complex, too. For example, you could reduce:
.Text = "([0-9A-z])(^9){1,}(^13){1,}"
.Replacement.Text = "\1\3"
to:
.Text = "^t@^13"
.Replacement.Text = "^p"
and you could reduce:
.Text = "([:])( )([0-9A-z])"
.Replacement.Text = "\1 \3"
to:
.Text = ": ([0-9A-z])"
.Replacement.Text = ": \1"

[Frosty]

You could also set up a repeatedly called sub routine which encapsulates the code you're calling... like so:


Sub MyFindReplace (cRng As Range, sFindText As String, sReplaceText as String)
With cRng.Find
.Forward = True
.Text = sFindText
.Replacement.Text = sReplaceText
.MatchCase = True
.MatchWildcards = True
.Wrap = wdFindContinue
.Execute Replace:=wdReplaceAll
End With
End Sub

Which you could then call repeatedly... and then ultimately put all your values into an array.

But when you say you want to "streamline" you have to define what that really means. Processing speed? Debugging speed (in terms of stepping through the code while it runs)? Ease of making further modifications?

If you are always matching the case, and matching wildcards and replacing all -- but the search and replace terms are constantly changing, then encapsulating that functionality into a single sub routine you call with only the things that change is the way to go.

So many options in the "streamlining" concept... but I wouldn't worry about processing speed. You're already using the find object, so that's good enough.

[macropod]

You could also set up a repeatedly called sub routine which encapsulates the code you're calling... like so:
...
Which you could then call repeatedly... and then ultimately put all your values into an array.
True, but re-setting all those options for each iteration is inefficient. Rather more efficient would be:

Sub Demo()
Application.ScreenUpdating = False
Dim StrFnd(), StrRep(), i As Long
StrFnd() = Array("^t@^13", ": ([0-9A-z])", "±")
StrRep() = Array("^p", ": \1", "plus or minus")
With ActiveDocument.Range.Find
.ClearFormatting
.Forward = True
.MatchWildcards = True
.Wrap = wdFindContinue
.Replacement.ClearFormatting
For i = 0 To UBound(StrFnd)
.Text = StrFnd(i)
.Replacement.Text = StrRep(i)
.Execute Replace:=wdReplaceAll
Next
End With
Application.ScreenUpdating = True
End Sub
Note: The only caveat with the above is that you need to have the same number of Find and Replace expressions in the arrays, whereas a linear process might simply re-use the last-used expression.

[Frosty]

That's true, which is why I talked about what "stream-lining" actually means for the OP. The "inefficiency" in setting 5 properties on the Find object is only one if you are stepping through the multiple operations. I don't believe that inefficiency has any effect on the actual performance of the code. From a development perspective (in terms of adding find/replace terms), I think it would be much more "efficient" to be able to simply call the same function over and over. However, if you believed (or tested) that over 1000s of operations, setting those particular properties multiple times actually had a performance hit... then you could pass the find object to the sub routine too...

I don't think it would be necessary, though. For me, the true "efficiency" here is in being able to easily add/remove/comment out specific find/replace pairs.

Separating out the actions of this kind of routine in order to be able to easily re-use parts of the code without having to revamp others is the key to efficiency: separate out the "data" (my "fGetFindReplaceTerms" function) from the setting up of the FindReplace function (my "FindReplace" routine) and also separated from the actual doing of the code within a particular document.

But I suspect "DemoSimple" is probably what the OP would benefit most from (as well as understanding how to use SHIFT+F8 when stepping through code)



'simple method, reusing the same procedure over and over, with different find/replace pairs
Sub DemoSimple()

FindReplace "^t@^13", "^p"
FindReplace ": ([0-9A-Z])", ": \1"
FindReplace "±", "plus or minus"


End Sub
'more complicated, re-using a find object, and an array from a separate function
Sub DemoComplex()
Dim i As Integer
Dim aryFindReplace() As String
Dim oFind As Find

'get our find/replace terms array
aryFindReplace = fGetFindReplaceTerms

'loop through them, until we have an "empty" find string
For i = LBound(aryFindReplace) To UBound(aryFindReplace)
If aryFindReplace(i, 0) = "" Then
Exit For
Else
FindReplace aryFindReplace(i, 0), aryFindReplace(i, 1), ActiveDocument.Content, oFind
End If
Next

End Sub
'single place for the data (could do this as a table in the macro project too)
Function fGetFindReplaceTerms() As Variant
Dim aryRet(10, 1) As String

aryRet(0, 0) = "^t@^13"
aryRet(0, 1) = "^p"

aryRet(1, 0) = ": ([0-9A-Z])"
aryRet(1, 1) = ": \1"

aryRet(2, 0) = "±"
aryRet(2, 1) = "plus or minus"

fGetFindReplaceTerms = aryRet
End Function
'simple procedure for repeating the same kind of find/replace, just with different find/replace terms
Sub FindReplace(sFind As String, sReplace As String, Optional cRange As Range, Optional oFind As Find)
'if no range passed, search the main text story of the active document
If cRange Is Nothing Then
Set cRange = ActiveDocument.Content
End If

'if no find object passed, then set it up
If oFind Is Nothing Then
Set oFind = cRange.Find
With oFind
.ClearFormatting
.Forward = True
.MatchWildcards = True
.Wrap = wdFindContinue
.Replacement.ClearFormatting
End With
End If

'replace all with the passed find/replace terms
With oFind
.Text = sFind
.Replacement.Text = sReplace
.Execute Replace:=wdReplaceAll
End With

End Sub

[Frosty]

There may be a conceptual issue with the passing of the find object if the search range was something other than the entire document doing a replace all (since a range is redefined on a successful search, and if you kept passing the same find object it might get a little wonky)... but I haven't fully wrapped my mind around that, as I doubt it would ever be truly worth doing.

I think re-using the code and removing find/replace pairs trumps the coding efficiency of Paul's demo routine (which would be very tricky to turn into a list of 50 items where you later wanted to remove items 25 and 43 -- you'd have to be very careful to count to 25 in each separate array and remove just that pair of terms.

This is not to say Paul's code is bad -- it's not, it's very good. But I think it's more inflexible and prone to a mistake when making modifications to the data rather than the programming. For me, that is the more important "efficiency" here. Of course, this is all just my opinion. Both answers are right.

[lkpederson]

My definition of "efficiency" is removing duplicates and streamlining the code. I figured you folks would have ideas on how to do that and as an added bonus, teach myself something new.

Thank you.