Solved: Problem with .find, wdextend, and delete

[mantooth29]

Below is a section of code that comes at the end of a loop. I am having problems when the Else section is triggered. If triggered, I need the program to delete this section without touching text elsewhere.

I have tested how it selects the range, and it works correctly by itself. However, somewhere between deleting the text, and coming back around to the beginning of the loop a whole sections is being deleted that should not be. I suspect this has something to do with the range extend property (which I need), and the find at the beginning of the loop.

Can anyone tell me what is wrong here?

End of Loop:


If IsEmpty(ve.Range("F10")) = False Then


With Selection
With .Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = "Doc4"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.Format = False
.MatchWholeWord = True
.Execute
End With

.Range.Delete
vetbl.Copy
.Paste
xl.CutCopyMode = False

End With

Else
With Selection
With .Find
.Text = "For Audit Reviews"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.MatchWholeWord = True
.Execute
End With
.Extend
.Find.ClearFormatting
With .Find
.Text = "sent by email."
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindAsk
.MatchWholeWord = True
.Execute
End With
.Range.Delete
End With
End If

Next Cell

Beginning of loop:



For Each cell In ExcelRange

With Selection
'.Collapse
With .Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = "Doc1"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.Format = False
.MatchWholeWord = True
'.MatchWildcards = True
.Execute
End With
.Range.Delete


myList.Copy
.Paste
.Tables(1).Rows.Alignment = wdAlignRowCenter
xl.CutCopyMode = False

End With

[Frosty]

It's always dangerous to base an entire routine around performing a .Find operation, and then not testing to see if the .Execute method is successful before performing actions like deleting text (.Execute will return True if it is successful, .Found after an .Execute will as well).

It's very tough to give you specific advice when you give only parts of the code, rather than the entirety of the routine. I appreciate you trying to give it in palatable chunks, but I suspect that if you knew where the actual problem was, you wouldn't post at all. Since you don't know where the problem is, that means it could be anywhere in your code.

I don't specifically see anything wrong in this code chunk. But I do see a whole lot of variables I have no idea what they are supposed to be or do: xl, myList, ve, vetbl, Cell, ExcelRange. And your use of .Range("F10") which suggests to me that at least some part of this is in Excel.

Are those dimmed variables? Are you using Option Explicit?

In general, if you feel the need to show a chunk of code because showing all of the code seems like too much: that's a good time to modularize your routine.

Create a sub routine which does *only* the thing you want it to do. Comment that subroutine (i.e., "Select all text between 'For Audit Reviews' and 'sent by email.' and delete it all"). Once you type that out, you'll realize you need to deal with scenarios where one or both of your phrases is not found for some reason.

Also, when you're using the .Find object, it is going to be a heck of a lot better to use a Range object rather than a Selection object. Look up "working with ranges" in the help file for more on the subject.

Apart from that, in order to help you specifically, you're probably going to need to say what you want to have happen. As clearly and simply as you possibly can. That starts by commenting within your code. A lot of bugs can be avoided simply by commenting what you expect (and want) to have happen.

[mantooth29]

I appreciate the tips Frosty. I can vouch that debugging all these finds is a real pain, but I had the code working as expected until I added the Else section for the if statement. And again, the Else section will work as a seperate module (without an if qualifier).

I can definitely appreciate that the range find is better than selection, but selection is the one I was able to get working:dunno. I am good with Excel, but am still working on Word VBA and Office Integration.

I have dimmed all the variables, although I do not use Option Explicit.
I have posted the code in its entirety below. And yes, it is a bit lengthy.



Dim xl As Excel.Application, wb As Excel.Workbook
Dim SList As Excel.Worksheet, jy As Excel.Worksheet, ctrl As Excel.Worksheet, ve As Excel.Worksheet
Dim person As Excel.Range, mgr As Excel.Range, jybox As Excel.Range, jyaccts As Excel.Range, jyactv As Excel.Range, SampleList As Excel.Range
Dim Filepath As Excel.Range, PDF As Excel.Range
Dim a As String, b

Dim myItem As Outlook.MailItem, objInspector As Outlook.Inspector
Dim myAttachments As Outlook.Attachments

Dim wordpaste As Selection, objDoc As Document

Set xl = CreateObject("Excel.Application")
xl.Visible = True

Set wb = xl.Workbooks.Open("C:\AFilepath\Emailer.xls", CorruptLoad:=XlCorruptLoad.xlRepairFile)
Set ctrl = wb.Worksheets("Control")
Set SList = wb.Worksheets("Sample List")
Set jy = wb.Worksheets("jy Box")
Set ve = wb.Worksheets("myTable")

Set mgr = ctrl.Range("A7")
Set mgr = ctrl.Range(mgr, mgr.End(xlDown))


Set jybox = jy.Range("jy123")


Excel.Application.DisplayAlerts = False



With wb.Worksheets("Control")

For Each person In mgr

'---- Begin executing code for each manager

ctrl.Range("B7") = person.Text

'---- Advanced filter calls

xl.Run ("Form")
xl.Run ("Filter")
xl.Run ("jyFilter")
xl.Run ("SE")


Set SampleList = SList.Range("F1:O1")
Set SampleList = SList.Range(SampleList, SampleList.End(xlDown))

Selection.HomeKey Unit:=wdStory


With Selection

With .Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = "Doc1"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.Format = False
.MatchWholeWord = True
.Execute
End With
.Range.Delete


SampleList.Copy
.Paste
.Tables(1).Rows.Alignment = wdAlignRowCenter
xl.CutCopyMode = False

End With


'-------Insert PDF's (This section will be revised once I solve the delete dilemma)
With Selection
With .Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = "Doc2"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.Format = False
.MatchWholeWord = True
.Execute
End With
.Range.Delete

Set Filepath = ctrl.Range("F7")
Set Filepath = ctrl.Range(Filepath, Filepath.End(xlDown))

If IsEmpty(ctrl.Range("F7")) = False Then
For Each PDF In Filepath
On Error Resume Next
a = PDF.Text
b = PDF.Offset(0, 1).Text

.InlineShapes.AddOLEObject ClassType:="AcroExch.Document.7", _
FileName:=b & ".pdf", LinkToFile:=False, _
DisplayAsIcon:=True, IconFileName:= _
"C:\Windows\Installer\_PDFFile.ico" _
, IconIndex:=0, IconLabel:=a & ".pdf"

Next PDF
End If

End With

'------jy table copy


Set jyaccts = jy.Range("I11")
Set jyaccts = jy.Range(jyaccts, jyaccts.End(xlDown))

With Selection
With .Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = "Doc3"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.Format = False
.MatchWholeWord = True
.Execute
End With
.Range.Delete
.TypeParagraph

If IsEmpty(jy.Range("I11")) = False Then '<> "" Then

For Each jyactv In jyaccts
jy.Range("Y2") = jyactv.Text
jybox.Copy

.Paste
xl.CutCopyMode = False

.TypeParagraph
.TypeParagraph

Next jyactv
End If
End With


'------Copy vetbl


Set vetbl = ve.Range("V2:AB2")
Set vetbl = ve.Range(vetbl, vetbl.End(xlDown))

If IsEmpty(ve.Range("F10")) = False Then

With Selection
With .Find
.ClearFormatting
.Replacement.ClearFormatting
.Text = "Doc4"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.Format = False
.MatchWholeWord = True
.Execute
End With

.Range.Delete
vetbl.Copy
.Paste
xl.CutCopyMode = False

End With
'=================================
'========The Problem code?========
'=================================
Else
With Selection
With .Find
.Text = "Some Sample Text"
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindContinue
.MatchWholeWord = True
.Execute
End With
.Extend
.Find.ClearFormatting
With .Find
.Text = "through an email."
.Replacement.Text = ""
.Forward = True
.Wrap = wdFindAsk
.MatchWholeWord = True
.Execute
End With
.Range.Delete

End With
End If

'=================================
'=================================
'=================================

Set SampleList = Nothing
Set vetbl = Nothing
Set Filepath = Nothing
Set jyaccts = Nothing
Set wordpaste = Nothing
Set myItem = Nothing
Set objInspector = Nothing



Next person

End With


Excel.Application.DisplayAlerts = True





I will work on modularizing this. In the meantime, if anyone has more ideas it would be appreciated!

[Frosty]

Couple of quick pointers...

1. Make sure to include your Sub name and End Sub...

2. Prefix your variables with prefixes that also indicate the datatype. And use meaningful prefix names... Especially when working in a subroutine which uses two different applications. Makes it a lot easier to know what is going on. So "ve" could become "wsMyTable" -- and then when you're reading the code it's a lot easier to debug, just by reading it.

3. Put Option Explicit at the top of your code module. Always use it. It will save you more often than you know-- especially if you always dim your variables.

4. Be really really clear what you're trying to operate on. Why do you use CreateObject to create a new instance of the Excel application, set it to an Excel.Application variable called "xl" ... and then have code which references Excel.Application? That can work, but it's pretty sloppy.

5. When using CreateObject, you should almost always be using GetObject first (in case there is already an instance running). I don't see where you kill the Excel application process here... if you run this procedure a lot, do you end up with 20+ Excel.exe processes in your Task Manager?

6. Can you comment more clearly what you want to have happen on these areas in your code? Without a sample spreadsheet, I have no idea how your data is set up. I can see that you're going through an excel spreadsheet, finding some stuff, copying it, and then pasting it into a word document-- but it's all very nebulous. Can you simplify the process a little?

It looks to me like you want to take the contents of the clipboard (the fact that you get that info from Excel doesn't really matter), and paste them into certain locations in a word document. And those areas are (presumably) defined by some beginning and some ending text. Yes?

If that problem code works on its own, but not within the routine... then it's likely that your Selection.Find object is not nearly as "clean" by the time you get to it as you think it is.

Try adding the following procedure to your code module:

'use the find operation to return a range defined by beginning text and end text
'NOTE: this could easily be a wildcard search, however, I've left like this to demonstrate something
'closer to your original structure
Public Function fGetRange(sBeginningText As String, sEndText As String, Optional rngSearchIn As Range) As Range
Dim rngSearch As Range
Dim rngReturn As Range

'if we passed the parameter, use it -- default to entire document
If rngSearchIn Is Nothing Then
Set rngSearch = ActiveDocument.Content
Else
Set rngSearch = rngSearchIn.Duplicate
End If

'find our beginning tetx
With rngSearch.Find
.Text = sBeginningText
.MatchWholeWord = True
'if the execute returns true, the rngSearch is modified to the found range
If .Execute = True Then
Set rngReturn = rngSearch.Duplicate
End If
End With

'adjust our search, to start from the end
rngSearch.Collapse wdCollapseEnd

'if no passed parameter
If rngSearchIn Is Nothing Then
rngSearch.End = ActiveDocument.Content.End
Else
rngSearch.End = rngSearchIn.End
End If

'find our end text
With rngSearch.Find
.Text = sEndText
If .Execute = True Then
rngReturn.End = rngSearch.End
'since we only set it here, if both terms are not found, then it will return nothing
Set fGetRange = rngReturn.Duplicate
End If
End With

End Function


And then replace all the code in the programmatic else chunk with...

Dim rngWorking As Range

Set rngWorking = fGetRange("Some Sample Text", "through an email.")
If Not rngWorking Is Nothing Then
rngWorking.Delete
End If

There is a better way to perform the find operation (using a wildcard search), but that might introduce complexities when you adapt this code (especially if you try to use Selection).

I think, in general, the problem you're having is with the use of the Selection object throughout your routine. When you do multiple .Find operations on the Selection, it "remembers" the past find operations. But when you use With blocks, you get a combination of snapshots of an object as it existed when you started the With block, and what you get returned to you. In short, giving you a full explanation is probably going to be a little overwhelming. Suffice to say that use of the Selection object is not a good practice except when you have to.

And especially the way you're using it in this one massive routine is going to be problematic, because you're building a house of cards (since each subsequent Selection.Find may or may not be affected by a previous Selection.Find... and you will get different results on successful searches than you will on unsuccessful searches). In addition, you have a decent chance of corrupting the entire .Find object over time, so you'd need to restart Word in order to continue.

[mantooth29]

What else can I say but.. Thank You! :biggrin:

It is so funny you used the term house of cards, because that is what it runs like! From the beginning, asking this of Word has proved challenging. I will try to convert the code into a range search instead of selection based. Will that change the dynamics of deleting?

I appreciate the helpful feedback and will post back when I've had time to implement your suggestions.

[Frosty]

Well, no need to thank me yet. This is only pointing you in the beginning of the right direction (in terms of moving away from Selection.Find operations and into Range.Find operations, and also modularizing your code a bit).

In terms of that actual function-- it's not ultimately necessary in that exact structure, I don't think. I *think* it's a good launching off point for working with ranges. You can step through that function, and periodically use the immediate window (CTRL+G) and type (without quotes)
rngSearch.Select
rngSearchIn.Select
rngReturn.Select

So you can see what the ranges are, and how they operate.

However, this is probably a more useful (and efficient) way to write that function. And it would allow you to re-use at various points in your code...

'-----------------------------------------------------------------------------
' Return a range defined by:
' 1. the beginning text passed
' 2. AND any text up to and including the end text passed (if any)
' Returns NOTHING if doesn't find matching phrases
' NOTE: searches the entire document if no SearchIn range is passed
'-----------------------------------------------------------------------------
Public Function fGetRange(sStartText As String, _
Optional sEndText As String, _
Optional rngSearchIn As Range) As Range

Dim rngSearch As Range

'if we passed the parameter, use it -- default to entire document
If rngSearchIn Is Nothing Then
Set rngSearch = ActiveDocument.Content
Else
Set rngSearch = rngSearchIn.Duplicate
End If

With rngSearch.Find
'if we didn't pass in any end text, we just search for that
If sEndText = "" Then
.Text = sStartText
'otherwise, we do a wildcard search to find that text
Else
.Text = sStartText & "*" & sEndText
.MatchWildcards = True
End If
'if the execute returns true, the rngSearch is modified to the found range
If .Execute = True Then
'now return it (remember, if no good search, this function returns nothing)
Set fGetRange = rngSearch.Duplicate
End If
End With

End Function


But for that particular functionality, it is simpler to simply perform a wildcard search, with the find text being (without quotes)
"Some Sample Text*through an email." Of course, I could have said that earlier.... but wildcard searching is an entirely different area. You should try it out in the regular find/replace dialogs before trying to use it in code.

And there are even more problems with implementing a wildcard search while you're still using the Selection object.

I think the key is figuring out a good *re-usable* function (or even a chunk of code) that you can pass a couple of parameters to (first phrase, second phrase (if any)) and have it return a range when it finds all the criteria you care about. And then you have to decide what you want that function to do when it doesn't find stuff (which is going to happen at times).

The nice thing about having that be a separate function means you start with a "clean" find object each time, because you're starting with a new range variable each time... as opposed to the same old Selection object throughout an entire routine.

If you gave me the excel spreadsheet and the word document, I could do it all for you in probably less time than it has taken for me to type this stuff.

However, I don't think you'd learn as much. Hope this helps...

[mantooth29]

Let me just say thanks again Frosty. This procedure runs much more smoothly using the range.find rather than selection. Your analysis has given me many insights and helped improve this routine quite a bit.

I was able to reproduce what I want the routine to do exactly, but I am still using Select method to get the Else section to work. I believe your function will be very useful in this procedure and future word endeavors, but I could still use some advice on adapting it to my needs. The function itself works nicely, but I haven't figured out how to adapt it in the context of this project.

I will explain exactly what I need the function to do (although you were able to infer quite a bit from the code alone).

High Level:
This is a mail merge output document. Once I am done testing, I will use this Doc as a template, and change the code to reference active document when performing the actions. I have set up markers for attachments and tables in the document ("Doc1","Doc2",etc). Each record has these markers.

If a condition is true in the excel file, I want to add a table or attachment and delete the marker. If it is false, I want to delete a specified section of text in that record alone. The marker is typically sandwiched between the range of text to be deleted if the Excel range evaluates to false.

When using your function, I need to figure out how to define the rngSearchIn parameter appropriately. As it stands, the function will go to the beginning of the document (first record), and delete sections that had a table or attachment added.

What I need the function to do (ultimately for multiple find sections in the code) is find the marker ("Doc4") which has not been deleted because it tested false, and is sandwiched between the range of text I want to delete. Then, go up a few paragraphs (or find in the reverse direction) and redefine the rngSearchIn for that section alone. Otherwise it defaults to the first instance of the text in the document.

Here is the code I was able to get working, but again I think understanding how to adapt your function would be much more useful. Especially since I have to apply the below behavior to several sections.




Else
findText = "Doc4"
rng.find.Execute (findText)
rng.Select
Selection.MoveUp Unit:=wdParagraph, Count:=6
Selection.Extend
Selection.find.Execute ("by you via email.")
Selection.Delete

End If



Thanks again for all the advice. I have learned a lot already :thumb

[Frosty]

Well, I would need to know a lot more to give you an exact answer. I suspect you may be working on a coding solution because you are learning mailmerge functionality on the fly.

It sounds like you're trying to perform cleanup on a mailmerge output document because some of the data you might reference in the data source doesn't exist for a specific record... is that accurate?

Because if it is... I *believe* there are built-in mail merge functions (IF codes, or something) which allow you to skip trying to spit out sections of data. However, I've never had to do a lot of work on mail-merge, and I know the versions of Word differ a good bit on functionality in that area.

That said... I don't know the criteria by which you define your selection (moving up 6 paragraphs, etc), but you're right, the function I wrote will either search the entire document or it will be constrained by the range you pass it.

So, if you do...
set rng = fGetRange("Doc4")
It will return a range which contains the first instance of the text "Doc 4"
however, if you do...
Set rng = fGetRange("Doc4", , Selection.Range)
then it will return a range which contains the first instance of the text "Doc 4" *contained in the selection.range you have passed*
Set rng = fGetRange("by you via email.", "Doc4", Selection.Range)

However, you need to get your selection.Range bigger before you pass it to the function, or it won't find anything, right?

So maybe, before you pass it Selection.Range... you do...
Selection.MoveStart wdParagraph, -6
Selection.MoveEnd wdParagraph,6
(that would extend your selection backwards 6 paragraphs, and forwards 6 paragraphs)
Or you can use
Selection.MoveUp (and move down) wdParagraph, 6, true
You'll see the difference in this functionality vs. the other. Try it out in the immediate window to see all the options.

If you start to understand this stuff, then you'll see that you can move away from the selection entirely. If you learn how to use the Selection well, you're actually well on your way to learning about ranges (especially since modifying the selection gives you a nice visual cue).

What you want to try to avoid, however, is simply recording macros and then copying/pasting them as chunks into code. That will send you down a path which is tough to troubleshoot.

Your else block could use a range exclusively... simply by removing the whole need to select things. You can delete a range which isn't selected...

I'm sorry I can't provide you exact code on how to use the function-- but I don't know how your document is set up. You can post a sample doc and sample data set, and perhaps someone more familiar with mail merges can give you some pointers. I'm trying to help you understand the programming better-- but I suspect you don't actually need programming for what you want to accomplish.

[mantooth29]

I have played with the advanced catalog merge and IF fields, and yes they are a pain. I will have to delve more into that for another project.

But for this email, I actually need Excel graphics (not charts per se) pasted into the document, and they are custom for each record. I hate using select, and never resort to it with Excel. But I am clumsy with word programming and am still getting used to its range properties. In fact, I used the select method because I found that moveup wdParagraph was not working properly otherwise.

I am guessing I could still use that IF field to determine if a section should exist, but I am trying to avoid going down that road for now.

I will work on implementing your suggestions to the function, and research Word's help file a little more before posting back.

[Frosty]

Check out .MoveStart ... the main difference you'll have with Selection navigation vs. Range navigation is if you're dealing with tables and table cells.

Since tables get "read" left to right... the .MoveUp / .MoveStart functionality can be different by a good bit when you're dealing with even the Selection.Range vs the Selection.

For an example, take a 4x4 table with some text in each cell... select the middle two columns. Then in the immediate window try Selection.Font.Bold = True vs Selection.Range.Font.Bold = True to see the difference.

I'm not sure how you identify your individual sections, but it might make it a good bit easier to set up your template in such a way that you clearly mark the beginning and end of areas, so that you don't have to do so much guess work in the macro. Put in fields which get populated if data doesn't exist in your excel spreadsheet, and then put them at the beginning and end (rather than the middle).

Something like
Doc4Sec1Start
yada yada
Doc4
yada yada
Doc4Sec1End

The you could use the macro to simply find everything between Doc4Sec1Start and Doc4Sec1End.

If you name these areas individually, you should be able to not worry about using your selection as a pseudo-progress bar.

If I recall correctly-- don't mail-merge documents end up as multiple sections? You could put all of your cleanup into a "For each oSec in ActiveDocument.Sections" loop, perhaps?

[mantooth29]

That did the trick regarding range navigation!

Here is the working code now.


Else
rng.find.Execute (findText)
With rng
.MoveStart Unit:=wdParagraph, Count:=-6
.MoveEnd Unit:=wdParagraph, Count:=3
.Delete
End With



Much cleaner.

Extending the range using these parameters works fine for my purposes, and I can still use your function for other parts of my code that are less convoluted.

I am still concerned about keeping the find function clean though. I am only testing this on 4 records, but eventually it will handle up to 25 or 30.
I may attempt to modify your function and pass those Long variables to the rngSearchIn parameter if you think it is worthwhile.

Thanks for the heads up on mergesections object. Potentially a big time saver there with the mail merge sections loop. Actually, if I applied that it could simplify the problem I was having with your function too, right? Could I just change the look in range to active section?

I will have to experiment.

Anyway, you have given me plenty to think about and I appreciate all your follow up. My primary problem has been resolved, and I'm keeping my fingers crossed that a new one doesn't take its place.

Thanks again, Frosty!

[mantooth29]

Thanks again to Frosty for suggesting a function. I have adapted his to resize the find range by a specified number of paragraphs in either direction. I may also modify this so that the Units are a parameter as well, but since paragraphs work well for my purposes now I will stick to that.

Here is the function:


Option Explicit

Public Function fGetNewRange(marker As String, _
Optional lngMoveUp As Long, _
Optional lngMoveDown As Long) As Range

Dim rngSearch As Range
Set rngSearch = ActiveDocument.Content


With rngSearch.find
.Text = marker
.Execute
End With
With rngSearch
If lngMoveUp <= 0 And lngMoveDown >= 0 Then
.MoveStart Unit:=wdParagraph, Count:=lngMoveUp
.MoveEnd Unit:=wdParagraph, Count:=lngMoveDown
ElseIf lngMoveUp = 0 Then
.MoveEnd Unit:=wdParagraph, Count:=lngMoveDown
ElseIf lngMoveDown = 0 Then
.MoveStart Unit:=wdParagraph, Count:=lngMoveUp
Else
MsgBox "Range is restricted to FindText"
End If
.Delete
End With


End Function



And the function applied....


Sub testfunction()

Dim rngWorking As Range
Set rngWorking = fGetNewRange("Doc4", -5, 3)

If Not rngWorking Is Nothing Then
rngWorking.Collapse
End If

End Sub

[Frosty]

that's good progress.

Also, know that you can give different defaults to optional parameters than what a new variable would be (so your lngMoveUp and lngMoveDown can be something other than 0, although I think that's appropriate in this case). You can also use built-in word enumerators as arguments (more on that in a second).

Big concept issue with your function as written is that you're not returning anything. So it will *always* return nothing. Thus your demo of its use is flawed.

Conceptually, there is a difference between a function and a sub-routine. A function can do stuff and also returns a value... a sub-routine only does stuff. This is kind of an over-simplification, since they both can return values by changing passed parameters. But for these purposes, my point is that your fGetNewRange function is inappropriately named. It is not getting a range. It is deleting a range. And it is also not returning that range (deleted or not).

Within the function, you're also making it a little more complicated than it needs to be. You don't need to have that else block. And I would either change the name of the function (fDeleteRange) or move the delete outside of the function and retain the name.

Here is a slightly simplified version of your function... however, be warned that it returns the ENTIRE document if the find fails (if your marker variable isn't returned).


Public Function fGetNewRange(strMarker As String, _
Optional lngMoveUp As Long, _
Optional lngMoveDown As Long, _
Optional lngUnit As WdUnits = wdParagraph) As Range

Dim rngSearch As Range
Set rngSearch = ActiveDocument.Content

With rngSearch.Find
.Text = strMarker
'this is true only on a succesfful search
If .Execute Then
'our defaults are wdParagraph and 0 (which leaves the range alone)
rngSearch.MoveStart Unit:=lngUnit, Count:=lngMoveUp
rngSearch.MoveEnd Unit:=lngUnit, Count:=lngMoveDown
'return the range of a successful search only?
Set fGetNewRange = rngSearch
End If
End With
'return the entire document here, if the search failed?
Set fGetNewRange = rngSearch
End Function


If you choose to put the .Delete method back into the fGetNewRange function, I would change the name of the function. And you should also know that when you delete range, it is not necessary to collapse it. It is already collapsed.

Hope that helps!

[Frosty]

Oh, and in the spirit of choosing the right names for things. One more quick note...

Naming a parameter "MoveUp" suggests a usage of the .MoveUp method. But you're using the .MoveStart method. Why does this matter? Because of the values you need to pass.

Selection.MoveUp wdParagraph, 1
will move the selection up one paragraph
Selection.MoveStart wdParagraph, -1
will move the selection up one paragraph

See where I'm going? You've made a counter-intuitive function by mixing up your parameter name with the method used... which leads an experienced programmer to initially pass the wrong values (I pass "1" to the MoveUp variable, and it goes the "wrong" direction).

Naming the variables (and commenting) will save you lots and lots of time down the road. Think about them more than you think you should.

EDIT: I hope none of this comes off as discouraging or overly critical. I like taking the time to help someone who is willing to be helped (an attitude in woeful short-supply at times).

[mantooth29]

Yeah I noticed it was a little quirky, but would execute as expected when set up the way I had it. Now I understand the output of the function was meant to be a range, not an action on the range. So the output needed to be setting a range variable to the result of the function.

The function is more flexible when getting a range, so I will leave it that way. I only made one modification to the entire document default, so the function essentially does nothing if the find parameter is not found in the specified range.


If .Execute Then
rngSearch.MoveStart Unit:=lngUnit, Count:=lngMoveUp
rngSearch.MoveEnd Unit:=lngUnit, Count:=lngMoveDown

Set fGetNewRange = rngSearch
Else
Set fGetNewRange = Nothing
End If



If I use this function and range variable it is set to (rngWorking) repeatedly throughout the loop, should I reset rngWorking to nothing after each time it is used, only at the end of the loop, or not bother with it at all?


Also, what do you mean by word enumerators? I do see how you set up wdParagraph default, and understand the long variable will default to 0 unless otherwise specified.

This has been very helpful in my understanding of functions. In the past I have not used them often, as I couldn't quite understand the advantage over subs.

[mantooth29]

Not at all. I have more and more of these kinds of projects, and I want to have the best understanding I can of what I am doing.

This has been eye opening, and I can appreciate the advice on naming conventions.
Now I have a better understanding of the parameters on wdMoveUp vs wdMoveStart. I wasn't quite sure why positive numbers worked on one but not the other.

I see how it is useful for those that would help on the forum, for me when I have to revisit something like this down the road, and for those that may work on this after me!

Thanks again!

:friends:

[Frosty]

There is no inherent advantage of a function over a sub. Both *can* do the same thing, in terms of modifying values and returning them (although the process is different). It would probably be useful to read up on ByRef/ByVal and their usage in Sub Statement and Function Statement in the help file.

Consider...

Sub ExplainingTheDifference()
Dim sTest As String

'set up our parameter
sTest = "Original Text"
'and display it
MsgBox sTest
'modify it, by passing it byref to a sub routine
ModifyAString sTest
'display the modified ByRef value
MsgBox sTest

'reset to a new test
sTest = "Test Two"
'display it
MsgBox sTest
'run a sub routine using that value
ModifyAStringByVal sTest
'it isn't changed, because of ByVal
MsgBox sTest

'reset to a new test
sTest = "Test Three"
'display the results of the function using that value
MsgBox fModifyAString(sTest)
'and the display the value of the variable (also modified)
MsgBox sTest

'reset to a new test
sTest = "Test Four"
'display the results of the function using that value
MsgBox fModifyAStringByVal(sTest)
'but the variable was left alone
MsgBox sTest

End Sub
Sub ModifyAString(ByRef sStringVar As String)
sStringVar = sStringVar & " Modified"
End Sub
Function fModifyAString(ByRef sStringVar As String) As String
sStringVar = sStringVar & " Modified"
fModifyAString = sStringVar
End Function
Sub ModifyAStringByVal(ByVal sStringVar As String)
sStringVar = sStringVar & " Modified"
End Sub
Function fModifyAStringByVal(ByVal sStringVar As String) As String
sStringVar = sStringVar & " Modified"
fModifyAStringByVal = sStringVar
End Function


Basically, ByRef (the default when you don't prefix a variable with anything), means that any parameter you pass to a function OR a subroutine will be given back to the calling routine in whatever state your function/sub left it. This may or may not be desired at times.

But ByVal means the subroutine/function creates a copy, and then does stuff with it, but the calling routine won't see any of those changes.

It's tough to give a brief answer on when to use what. It depends on the scenario. There are also exceptions (certain complicated items can, if I recall correctly, ignore the use of ByVal or throw up an error when you try to use it -- mostly objects, classes, etc).

The short answer is that ByVal can be really useful when you're modifying strings in a subroutine, and if you have any errors... you can simply exit the routine and know that your original value hasn't been changed "halfway" because of some error in the middle of your subroutine.

As for the answer(s) to your direct question(s):

1. You don't need to explicitly set fGetNewRange = Nothing... because it will return nothing outside of the routine if you only set it in certain conditions (if .Execute = True) and those conditions aren't met.

2. If, in your loop, you are always setting your rngWorking = fGetNewRange ... then you won't need to set it to Nothing within the loop, because fGetNewRange will have done that for you.

3. Functions vs. Sub end up being simply a way of organizing and structuring your code. Since Functions *can* return values, I tend to let them return values. Whenever possible, I like to separate the "data" from the "doings" (that's how I always think of it). So, when I want to gather info/data (a range)-- I tend to use a function to get that info. If I want to "do" something with that info, I would generally use a subroutine. This is a major over-simplification, as I will often use a function to "do" a *lot* of stuff, but then return a value of whether it succeeded fully, partially, or not at all. Generally speaking, most of my complicated functions return a Long value, with a numeric indication of its success. My functions which return something other than a long, tend to be, regardless of complexity, organized around gathering information but not actually doing anything with it (so a function which returned a range would either return the "right" range or nothing at all).

4. Enumerations (both built-in and custom) are really useful constructs. Use of F2 to display the object browser can teach you about their use (and the actual values). All enumerations are technically "long" values, but they can make your coding a lot more readable.

Selection.MoveStart 4, -1
is a lot less readable than
Selection.MoveStart wdParagraph, -1
Even though they both do the same thing.

[mantooth29]

Hate to bump up an old thread, but was hoping to kill two birds with one stone.

1st) Thank you again, Frosty! This thread really got me thinking about the possibilities using cross application VBA, and definitely helped me grow as a programmer. I am grateful :bow:

2nd) I had to revisit this code recently, and came up with a simple function that uses Bookmarks as opposed to .find.text. The advantages of bookmarks is that anyone can define the sections to be edited without worrying how the text may change, or how many paragraphs to extend in either direction.

Edited version now works as expected
Function...


Function ExtendRange(ByRef bkmrkRef1 As Word.Bookmark, ByRef bkmrkRef2 As Word.Bookmark) As Word.Range
'Extends a Range from 1 BookMark to another with the same name.

Set ExtendRange = ActiveDocument.Content

With ExtendRange
.SetRange bkmrkRef1.Start, bkmrkRef2.End
End With

End Function

Use...(Errors are caught in calling code, when supplying an invalid bookmark)


Function testExtendRange()

Dim bm1 As Word.Bookmark
Dim bm2 As Word.Bookmark
Dim rngSelect As Word.Range

Set bm1 = ActiveDocument.Bookmarks.Item("Test1")
Set bm2 = ActiveDocument.Bookmarks.Item("Test2")

Set rngSelect = ExtendRange(bm1, bm2)

rngSelect.Select

End Function

Hope someone finds this useful!

[Frosty]

1) You're welcome
2) Bookmarks are hugely useful, however, be aware that normal use (and certainly when you select and then start typing) will wipe out the bookmark. Bookmark methodology is actually a good bit easier when translating form data into a document... but a lot less user-friendly if you are expecting bookmarks to be somewhere and they've been deleted over the course of normal editing of the document.

And, not to nag, but remember that your Function is still basically a subroutine-- it's not returning anything.

However-- another difference is that a Function won't show up in the list of Macros available from Word, whereas Public sub routines will (if they don't have any parameters).

[fumei]

Both of the Functions should be Subs as neither return anything.

[mantooth29]

Ahh thank you for pointing out the Function return error. I prefer to keep the helper routines as functions, as they can return a range and are not meant to be called directly by a user (for my purposes). Of course the example should have been a sub.

After further consideration, I am ditching the bookmark approach due to what you mentioned and the fact it does not survive a mail merge (I should have known better than to think this would be that easy).

I have devised a function that accomplishes my original intent: To find a placeholder, and then expand the range from that place holder in various directions using the find function (as opposed to predefined units).

It uses some of the original code provided by Frosty. For anyone who is interested, you can paste all this code into a standard module.


'For the ExpandRange Function
Public Enum wrdDirection

wrdUp = 1
wrdDown
wrdUpThenDown

End Enum

'Targets an inital Range
Function GetRange(srchTxt1 As String, Optional srchTxt2 As String = "", _
Optional ByVal srchRange As Word.Range = Nothing) As Word.Range

If srchRange Is Nothing Then Set srchRange = ActiveDocument.Content

With srchRange.Find
.ClearFormatting
If srchTxt2 = "" Then
.Text = srchTxt1
Else
.Text = srchTxt1 & "*" & srchTxt2
.MatchWildcards = True
End If
If .Execute Then Set GetRange = srchRange.Duplicate
End With

End Function

'Expands Range from starting point in multiple directions
Function ExpandRange(ByVal startRange As Word.Range, srchTxt1 As String, Optional srchTxt2 As String, _
Optional chooseDirection As wrdDirection = wrdDown) As Word.Range

'srchTxt2 is ignored unless chooseDirection = wrdUpThenDown

Dim startPlaceHolder As String 'to retain original text as the startRange is modified
startPlaceHolder = startRange.Text

With startRange.Find
.ClearFormatting
.MatchWildcards = True

If chooseDirection = wrdUp Then
startRange.Collapse wdCollapseEnd
.Forward = False
.Text = srchTxt1 & "*" & startPlaceHolder

ElseIf chooseDirection = wrdDown Then
startRange.Collapse wdCollapseStart
.Text = startPlaceHolder & "*" & srchTxt1

ElseIf chooseDirection = wrdUpThenDown Then
startRange.Collapse wdCollapseStart
.Forward = False
.Text = srchTxt1
If .Execute Then startRange.Collapse wdCollapseStart Else GoTo CleanExit
.Forward = True
.Text = srchTxt1 & "*" & srchTxt2

Else
Err.Raise 9, , "You must choose from the enumerated directions"

End If

If .Execute Then Set ExpandRange = startRange.Duplicate Else GoTo CleanExit

End With

Exit Function

CleanExit:

End Function



And then test the functions with something like...

'Sample use of both functions

Sub TestFunctions()

Dim srch1 As String, srch2 As String
srch1 = "Middle"
srch2 = "Sec1" 'In the sample doc, Sec1 is found TWICE. Once above middle, and once below
'The function was designed to allow the tags to be named exactly the same, or different.

Dim initialRange As Word.Range, expandedRange As Word.Range

Set initialRange = GetRange(srch1)
initialRange.Select

Set expandedRange = ExpandRange(initialRange, srch2, srch2, wrdUp)
expandedRange.Select

End Sub

[mantooth29]

Hate to bump this up again, but wanted to repost my code for anyone who may come across.

If used as in my previous post, the range being passed in as an argument ByVal will be modified regardless of the argument type, and regardless of the fact we are instructing VBA to .Duplicate the range.

I noticed this when using these functions in succession, ie...

Dim rng1 As Word.Range, rng2 As Word.Range, rng3 As Word.Range

Set rng1 = GetRange("MidPoint", , ActiveDocument.Content)

Set rng2 = ExpandRange(rng1, "TestStart", "TestEnd", wrdUpThenDown)

Set rng3 = GetRange("Mid", , rng2)

Debug.Print rng1.Text

Debug.Print rng2.Text

Debug.Print rng3.Text


Would result in rng2 overwriting rng1. This has to do with how Word processes ranges and even if ByVal is passed, the procedure can still modify the passed range. This question pointed me towards my conclusion -
http://stackoverflow.com/questions/7537886/clone-variable-in-word-vba

The fix is to create a temporary range variable inside the function, which seems to force VBA to make a copy of the variable. Because we are explicitly copying the passed range inside our function, it seems that reverting the argument to ByRef works fine. I'm not sure but I think doing so could save some small portion of memory.

Here is the edited code, which should retain the structure of ranges as they are created with the functions.

Public Enum wrdDirection

wrdUp = 1
wrdDown = 2
wrdUpThenDown = 3

End Enum

'Targets an inital Range
Function GetRange(SearchTxt1 As String, Optional SearchTxt2 As String = "", _
Optional ByRef srchRange As Word.Range = Nothing) As Word.Range

If srchRange Is Nothing Then Set srchRange = ActiveDocument.Content

Dim tempRange As Word.Range
Set tempRange = srchRange.Duplicate

With tempRange.Find
.ClearFormatting
If SearchTxt2 = "" Then
.Text = SearchTxt1
Else
.Text = SearchTxt1 & "*" & SearchTxt2
.MatchWildcards = True
End If
If .Execute Then Set GetRange = tempRange.Duplicate
End With

End Function

'Expands Range from starting point in multiple directions
Function ExpandRange(ByRef StartRange As Word.Range, SearchTxt1 As String, Optional SearchTxt2 As String, _
Optional ChooseDirection As wrdDirection = wrdDown) As Word.Range

'SearchTxt2 is ignored unless chooseDirection = wrdUpThenDown

Dim startPlaceHolder As String 'to retain original text as the startRange is modified
startPlaceHolder = StartRange.Text

Dim tempRange As Word.Range
Set tempRange = StartRange.Duplicate

With tempRange.Find
.ClearFormatting
.MatchWildcards = True

If ChooseDirection = wrdUp Then
tempRange.Collapse wdCollapseEnd
.Forward = False
.Text = SearchTxt1 & "*" & startPlaceHolder

ElseIf ChooseDirection = wrdDown Then
tempRange.Collapse wdCollapseStart
.Text = startPlaceHolder & "*" & SearchTxt1

ElseIf ChooseDirection = wrdUpThenDown Then
tempRange.Collapse wdCollapseStart
.Forward = False
.Text = SearchTxt1
If .Execute Then tempRange.Collapse wdCollapseStart Else GoTo CleanExit
.Forward = True
.Text = SearchTxt1 & "*" & SearchTxt2

Else
Err.Raise 9, , "You must choose from the enumerated directions"

End If

If .Execute Then Set ExpandRange = tempRange.Duplicate Else GoTo CleanExit

End With

Exit Function

CleanExit:

End Function