Solved: getting variables from 2 different comboboxes.

[SOS]

Hi all,

First post here.

I have a 2 tabbed userform (used by secretaries to send letters to either patients or doctors).

On this form I have a combobox (along with many otehr controls) named comStaffName which is populated with names of letter composers (staff names change depending on the value of combobox comTeam).

I also have a combobox named comJoint (which contains the same staff names and is used if two members of staff are going to be involved in the appointment).

I have a function to determine information about the staff using a Select case statement (ie position, tel no etc).

I need to obtain the same information from the other combobox comJoint and would like to know of any way to re-use that function because I don't want to have to write another version of it just to cope with items from comjoint.

If I haven't made myself clear I can give more info. I could also post a zipped version of the template but I am aware that it contains real data.

Many thanks for any help

Seamus

[fumei]

Not clear. Can you pass the combobox to the function as a parameter, therefore using the function for both?

It would help if you post some code.

Perhaps better yet, can you clear any data that you do not want to show, and post a sample file?

[SOS]

fumei,

zipped up the file having either taken real data out or replaced it with dummy data. I'd be most grateful if you'd have a look at it. I'd be really interested in any comments you may have.

That having been said, I'm quite embarrassed about letting my newbie coding abilities be seen!!

TIA

Seamus

[fumei]

Taking a look at it.

You have absolutely nothing to be embarassed about. Whatsoever. If you really are a newbie, then I must complimernt you on a quite sophisticated userform with rather good design. It is good. It is well thought out.

I am VERY impressed with the fact that you explicitly named all your controls. You are kidding about being a newbie, right? Newbies rarely, if ever, explicitly name controls.

Anyway...

I don't like the lengthy If..ElseIf statements. I have just taken a quick look, but generally I have found that many lines of ElseIf can usually be done better.

However, that is not the question, is it?

Let's try and clarify what exactly IS the question. I have re-read you OG and I am not clear.

I think it is related to your use of the same array repeatedly. For example, you have two arrays of Monday, Tuesday....

Why not use one array and load twice?

So, if I understand correctly, you are looking for a function to get similar/the same information out of two controls?

BTW: I totally hate the way you are constructing the letter. Bleeech. All those Selection instructions. Bleeeech.

Plus your use of Functions. While technically not incorrect, to me what you are doing with most of them would properly be Subs, not Functions.

Anyway, back to the real question. Please statement it again clearly. Now that I have the file hopefully I can grasp what you are asking better.

A (perhaps) good start would be to try and simplify your code. For example, here is your code for changes to comPTTeam. BTW: the normal prefix for a combobox is "cbo". I find myself getting distracted with the "com" prefix. Technically, you can use what ever you want. Here is your code:Sub comPTTeam_Change()
Select Case comPTTeam.Value
Case Is = "Medical"
With comPTStaff
.Clear
.List = Array("Davy Jones", "Micky Dolenz", _
"John Lennon", "Ringo Starr")
.ListIndex = 0
End With
Case Is = "Nursing"
With comPTStaff
.Clear
.List = Array("Karen Carpenter", "Matthew Perry")
.ListIndex = 0
End With
Case Is = "Occupational Therapy"
With comPTStaff
.Clear
.List = Array("Gene Wilder", "Austin Powers")
.ListIndex = 0
End With
Case Is = "Psychology"
With comPTStaff
.Clear
.List = Array("Michael Douglas", "Paris Hilton")
.ListIndex = 0
End With
Case Else
End Select
End SubWhen writing code in a procedure one of the first things to look for is repeated instructions that do not need to be repeated.

Look at the code above. In each Case, you .Clear, and in each Case you set ListIndex = 0. Here is the code again, although I used .Text instead of .Value. This means I do not have to use Is = :Sub comPTTeam_Change()
comPTStaff.Clear
Select Case comPTTeam.Text
Case "Medical"
comPTStaff.List = Array("Davy Jones", "Micky Dolenz", _
"John Lennon", "Ringo Starr")
Case "Nursing"
comPTStaff.List = Array("Karen Carpenter", "Matthew Perry")
Case "Occupational Therapy"
comPTStaff.List = Array("Gene Wilder", "Austin Powers")
Case "Psychology"
comPTStaff.List = Array("Michael Douglas", "Paris Hilton")
Case Else
End Select
comPTStaff.ListIndex = 0
End SubSee? As comPTStaff is .Clear in all Cases, then just do it first. As .ListIndex = 0 in all Cases, then just do it.

It may be better to use arrays. Declare them in your standard module as you can not declare Public arrays in an object module. But you CAN populate them within an object module. For example:

In the standard module (Module 1) Public MedStaff()

In the user form module:Sub PopulateThem()

MedStaff = Array("Davy Jones", "Micky Dolenz", _
"John Lennon", _
"Ringo Starr", "Karen Carpenter", "Matthew Perry", _
"Gene Wilder", "Austin Powers", "Michael Douglas", _
"Paris Hilton", "Gerry Knight")
' can do all possible arrys here
End Sub

Sub UserForm_Initialize()

Call PopulateThem
' NOTE, you do not need to use the With Me
' you can simply name the object
comJoint.List = MedStaff()
' all your other code
See? You can load the list directly from the array. As it is a Public array, you can use it anywhere you want, over and over again.

For example, going back to the comPTTeam Change event. Case "Medical"
comPTStaff.List = MedStaff()If your arrays are already done, then you can just use them. No need to write them in over and over.

Finally.....

Ahem......Use Option Explicit!!!!!!!

So, see if you can tighten up what you are doing with the one combobox, then try and use it again for the other. I am still not precisely clear on what you are trying to do.

Oh...and ......Use Option Explicit!!!!!!!

[fumei]

Oh, and if you really are a newbie - and frankly, I don't believe that at all - then you have done a superb job so far.

Except for not using Option Explicit.

Hmmmm, maybe you are a newbie.

[SOS]

fumei,

Absolutely kudos to you for such a detailed reply. I've only just read through it and will test and change my code tomorrow (10:45pm here now).

I am a newbie but I have messed around a wee bit before with VBA - almost all self-taught but a good portion of advice from various forums (or should that be fora) on the net and work colleagues.

My original question was (now that you have seen the form) is that from Select Case szStaff I can get the letter writer's position ie Davy Jones, Consultant. Now if chkJoint.value = true and a name is chosen from comJoint (OK it should be cboJoint) eg Karen Carpenter how could I then use the same Select Case statment to get her position ie Nurse.

Does that make sense?

Many thanks for your promt and courteous reply and I will respond tomorrow when I attempt to redesign the real data form.

Seamus

[SOS]

Gerry,
Once again, many thanks for the pointers re the above file. I have started to tidy it up as best I can - using OPTION EXPLICIT - etc.
Perhaps you'd be kind enough to have another look at it and give me some advice on two points.
1. Any advice on how the letters are constructed because you certainly seemed to abhor all the Selection stuff - bleeech.
2. Actually the original question. I use the sub Identify_Staff to get info on them ie Offical Name (Davy Jones becomes Dr D Jones), Position (Consultant), Tel No and Reference (DJ).
That's OK. But, if I use the cboJoint control to tell the sub that a second person will be attending the appt, is there any way of using that Sub Identify_Staff again to get info on the second person ie position, tel no and reference.
If a second person is attending the appt that line of the letter says "Your appointment will be with (szStaff) Dr D Jones, (szPosition) Consultant and Karen Carpenter."
I'd like it to be able to say "Your appointment will be with (szStaff) Dr D Jones, (szPosition) Consultant and (szStaff) K Carpenter, (szPosition) Community Nurse".
Does that make more sense?
Thanks again
Seamus
(Revised template attached)

[fumei]

I still getting to the main question....

May I suggest you use Styles? Word is designed around Styles. They are completely integral to it. You are using a Function (NewPara) - which again, I think should be a Sub - to add those horrid "extra" spaces between paragraphs.

They are NOT extra spaces. They are extra paragraphs. I never, EVER, have those "extra" paragraphs to make space between paragraphs. The space is built into the Style used for text. ElseIf opt30day.Value = True Then
With Selection
.TypeText "We note that you did not attend your outpatient " _
appointment on " & tbxOldDate & " with " & szStaff & _
" at Health Centre."
NewPara (2)
.TypeText "Please contact the above telephone number " _
& "if you would like to " & _
"arrange another appointment."
NewPara (2)
.TypeText "If we do not hear from you within 30 days (" & _
DateAdd("m", 1, Date) & "), we will assume that you no " & _
"longer wish to be seen and you will be discharged back " _
& "to the care of your General Practitioner."
NewPara (3)
.TypeText "Yours sincerely"
NewPara (8)
SignaturePT
End WithSee? You call NewPara four separate times. Here is the same effect, except I added Styles to your template. The spaces are built into the Styles. Therefore, for example, there is NO NEED to put in "extra" paragraphs to make the space between paragraphs. With Selection
.Style = "myMainText"
.TypeText Text:="We note that you did not attend your " _
& "outpatient appointment on " & tbxOldDate & " with " _
& szStaff & "at Health Centre." & _
vbCrLf & "Please contact the above telephone number " _
& "if you would like to arrange another appointment." _
& "If we do not hear from you within 30 days (" _
& DateAdd("m", 1, Date) & "), we will assume that you no " _
& "longer wish to be seen and you will be discharged " _
& "back to the care of your General Practitioner." _
& vbCrLf
.Style = "mySalutation"
.TypeText Text:="Yours sincerely" & vbCrLf
SignaturePT
End With

Here is the amended code for SignaturePT. With Selection
Select Case cboPTTeam.Value
Case "Medical", "Psychology"
.Style = "Composer"
.TypeText szComposer & vbCrLf
.Style = "ComposerTitle"
.TypeText "Appointments Secretary"
Case "Nursing"
.Style = "Composer"
.TypeText szStaff & vbCrLf
.Style = "ComposerTitle"
.TypeText szPosition
Case Else
End Select
End WithIn particular, I almost never use Selection.TypeParagraph, as a paragraph mark can be added to any string using vbCrLf. Why add an instruction when you do not need one?

Along the same lines, why do multiple TypeText instructions, when you can do it with ONE?.TypeText tbxTitle & " " & tbxForename & " " & tbxSurname & vbCrLf
.TypeText tbxAddress1 & vbCrLf
.TypeText tbxAddress2 & vbCrLf
.TypeText tbxAddress3 & vbCrLf
.TypeText tbxPostCode
versus
.TypeText tbxTitle & " " & tbxForename & " " & _
tbxSurname & vbCrLf & tbxAddress1 & vbCrLf & _
tbxAddress2 & vbCrLf & tbxAddress3 & vbCrLf & _
tbxPostCode

ONE instruction vs FIVE.


I'd like it to be able to say "Your appointment will be with (szStaff) Dr D Jones, (szPosition) Consultant and (szStaff) K Carpenter, (szPosition) Community Nurse".
Does that make more sense?Yes, absolutely.

So pass the names in to Identify_Staff as parameters.Sub Identify_Staff(sName1 As String, sName2 As String)
sName1 = szStaff as it is used currently.
sName2 = value from cboJoint

In the code for Identify_Staff, do what you are doing with szStaff - but use sName1.

Then, check the value of sName2. Previously put in code that checks chkJoint. If it is checked, then sName2 = cboJoint.Text. If it is NOT checked, then sName2 = "".

So, in Identify_Staff, if sName2 = "" (no joint name), then there is nothing to do. If it is not "" Select Case the value just like you do with your current szStaff.

Add the values you need into the procedure. I am putting together an updated file with this added.

[SOS]

fumei,

What can I say? I await the amended template with much interest and that in itself will teach me much more than floundering around learning a wee bit here and a wee bit there.

It's just that most of my VBA experience is in Excel and I've never really got to grips with styles in Word.

TIA

Seamus

[fumei]

BTW: overall, I am quite impressed by the work you have done on this. The error trapping for empty values is cleverly written.

I do not mean to be overly critical of the .TypeParagraphs, as you do, in fact, use vbCrLf in other places. Why not use wherever you can?

Another suggestion. Break up the code more. The procedure for cmdOK is very long. Rather than have all the code there if optNew.alue = True ( a "New" letter), put all the code for New in its own procedure.
If optNew.Value = True Then Call NewLetter
If Opt30Day.Value = True Then Call ThirtyDay

etc. etc.

That way, it is easier to focus on tasks.

[SOS]

fumei,

I have been working away at the suggestions you have made about styles and vbcrlf.

The code is much neater now but I'd be really grateful if you could explain a bit further about



So pass the names in to Identify_Staff as parameters.



VBA:



Sub Identify_Staff(sName1 As String, sName2 As String)



VBA tags courtesy of www.thecodenet.com (http://www.thecodenet.com/)



sName1 = szStaff as it is used currently.
sName2 = value from cboJoint

In the code for Identify_Staff, do what you are doing with szStaff - but use sName1.

Then, check the value of sName2. Previously put in code that checks chkJoint. If it is checked, then sName2 = cboJoint.Text. If it is NOT checked, then sName2 = "".

So, in Identify_Staff, if sName2 = "" (no joint name), then there is nothing to do. If it is not "" Select Case the value just like you do with your current szStaff.



I'm just struggling as to what exactly to do.

If I start the sub as


Sub Identify_Staff(sName1 as String, sName2 as String)


and I Select Case sName1, how do I then re-run that to Select Case sName2 for the second time around to get the Position of the szJoint person?

BTW, I've now split the Sub OK routine up as you suggested with the following


Select Case True
Case optNew: Call NewLetter
Case optFollowUp: Call Follow_Up_Letter
Case optCancelbyUsNewAppt: Cancel_By_Us_New_Appt_Letter
Case optCancelByUsNoAppt: Cancel_By_Us_No_Appt_Letter
Case optCancelbyPatient: Cancel_By_Patient_Letter
Case optChange: Change_Appt_Letter
Case opt30day: Call Thirty_Day_Letter
Case Else
End Select




Many thanks for your patience with me.

regards

Seamus

[fumei]

1. Add the values for the Joiner people into the procedure.

2. If Joiner has NOT been selected, then you make sName2 (or szJoiner if you prefer) = "".
Sub Identify_Staff(szstaff As String, _
szJoiner As String)
Select Case szstaff
' yadda yadda
End Select
Select Case szJoiner
Case ""
' no Joiner, do nothing
Case "Karen Carpenter"
szJointPosition = "Community Nurse"
Case "Ringo Starr"
szJointPosition = "Drummer"
Case Else
End SelectThe Joiner position must be a separate variable, as you are using separate values.

You will have to move the Call to each letter type procedure...which should be separate procedures anyway.

While, again, this is quite good work, I have a number of issues with it.

There is WAAAAAAY too much processing going on, IMO.

There should be use of Styles. I really, really, really, hate those Call BoldText procedures. ALL of them are totally pointless and can be dumped, if you use Styles.

Bottom line is this is good, but could be superb.

As for the Joiner position, I have modified the code to demonstrate doing this.

IMPORTANT!!! I have moved the Identify_Staff procedure (that now takes two parameters szstaff and szjoint) to the New letter part of the code ONLY. Repeat ONLY.

In other words, to see it work, ONLY use New letter and check Join.

All other letters will NOT work, as they no longer have the call to Identify_Staff.

Also note that I explicitly declare szstaff, and szjoint as strings. Plus, of course adding the new variable szJointPosition for the Joiner position.

[SOS]

Excellent fumei,

Just had a look and now understand (slightly better) the parameter passing. I've been doing quite a bit of work on it as well to cut down on the processing - eg, styles, vbcrlf and for example in the checking if all fields are completed I now only loop through either one page or the other of MultiPage1.

Many, many thanks for your time on this.

Regards

Seamus

[fumei]

Ah, excellent. Yes, all that checking is a major amount of looping.

All that error trapping should be moved out, into its own procedure.

Have you actually F8 this beast, to Step through it???

Aiiiieeee!

[SOS]

In a nutshell, though, what difference would moving the error trapping into its own procedure make. I mean it still has to loop through etc so does that help the whole thing?

[SOS]

I think I see what you mean. I've just moved the error trapping out into a sub of its own called Sub Check_PT_Letter_Fields so that the main Sub OK
should really only consist calling other smaller procedures and the text of the letter itself.

Is that a reasonable understanding?

Regards

Seamus

[fumei]

Yes.

Breaking code into chunks is good practice. It narrows down debugging immensely.

Technically, yes, instructions are instructions. Moving 100 instructions to a separate procedure does nothing in terms of reducing the number of instructions. But it DOES make a difference in understanding.

BTW: are you aware that you can switch a code module to a single selectable procedure, rather than ALL of them?

Say you wanted to have just the code for Sub cboGPTeam_Change()?

You can select that procedure with selecting cboGPTeam from the top LEFT dropdown, and Change from the top RIGHT dropdown.

To have it the ONLY code in the window, click the left-most button at the bottom left. There are two buttons.

The left one hides all procedures except the current one (ie. the one with the cursor in it). The right one shows all procedures.

This can be handy when you have a code module with a number of procedures.

[SOS]

fumei,

Thanks for that.

This is really good though


To have it the ONLY code in the window, click the left-most button at the bottom left. There are two buttons.

The left one hides all procedures except the current one (ie. the one with the cursor in it). The right one shows all procedures.

This can be handy when you have a code module with a number of procedures.

I think this thread could be marked as solved unless you or anyone else has any objection.

Once again - thanks for the help. I have learned quite a bit just from trying to put together a procedure to help the secretaries write consistent letters!

Regards

Seamus

[fumei]

Yup, I think it is solved.

BTW: I may keep playing around with your file, altering it to use Styles and bookmarks to create the letter. I suspect that the amount of code could be reduced by 40%. However, that is another issue, and not relevant to the thread.

If I do - and it would take a bit of work - I will post it.

[SOS]

No doubt I'll keep on tinkering with it - possibly having all the letters pre-fabricated as documents with bookmarks in them and then open them from the VBA code and populate the bookmarks with the necessary variables.

Your help has been invaluable

Marked as Solved

Regards

Seamus

[fumei]

possibly having all the letters pre-fabricated as documents with bookmarks in them and then open them from the VBA codeYES!!! YES!!!

I was hoping you would finally get to that.

That is exactly the way I work with multiple structures from one template. This writing directly with Selection sucks.

BINGO!

[SOS]

fumei

I tried a wee mock up of that last night - having a "New Letter" comprising of body text and bookmarks. In my code I then inserted a copy of "New Letter" (unsure whether to do that or have a "New Template" and base a document on that), and the fields all populated correctly.

I just need to do some more reading on bookmarks to make sure I'm using them correctly - eg I don't want the whole document to move down when I add Pt Name and Address variables to Recipient_Letter bookmark. Or should I have a bookmark for each varaible - like Recipient_Name, Recipient_Address1, Recipient_Address2 etc etc.


I was hoping you would finally get to that.

Quite pleased you didn't just give me that answer right away but instead let (made) me work it out for myself.

Regards

Seamus

[fumei]

I am grateful for your understanding. Yes, I could have mentioned that immediately, but in the long run it is MUCH better for you to have worked through the process.

In fact, generally speaking, I am reluctant to just hand over solutions. I am not getting paid for solutions. Come to think of it I am not getting paid at all!

So the reason I am in this forum - and others - is to both learn (hey I am selfish too), and assist others to learn. THAT is my return. When I see others actually understanding. It pleases me.

It has been a delight interacting with you on this one. You are going to be a good Word VBA person.

As for the issue of space - inserting text moving stuff "down" - yes, that can be a pain. All I can say is keep your options open. You may want to consider using tables (with no visible borders) for part of the document.

Often, the actual design structure of a document can change due to these factors.

[SOS]

fumei,

I have to say that I have now reworked the original code to open a doc based on a template for each letter and realised that virtually all letters use the same variables ie szStaff, cboOldDate, tbxNewTime etc and therefore the VBA code is tiny for each of the letters because all they use is Bookmarks!!

So the code now looks something like:



Sub New_Letter()
Documents.Add Template:="C:\New Letter.dot", NewTemplate:=False, _
DocumentType:=0
'separate sub used in every letter eg Forename Surname and Address etc
Call Enter_Patient_Details_Into_Letter
With ActiveDocument
.Bookmarks("New_Appt_With").Range.Text = szStaff & ", (" & szPosition & _
")"
.Bookmarks("New_Appt_Day").Range.Text = cboNewDay
.Bookmarks("New_Appt_Date").Range.Text = tbxNewDate
.Bookmarks("New_Appt_Time").Range.Text = tbxNewTime
.Bookmarks("New_Appt_Location").Range.Text = cboNewLocation
End With
'separate sub used in every letter for signing purposes
Call Sign_Patient_Letter
End Sub


Now I'm sorry to keep harping on (seeing as how this thread is supposed to marked Solved) but:

If there was a way to be able to list ALL the possible bookmarks in the sub above and have it ignore the ones that are not needed - then I could have just one sub for all letters. What do you think?

Like:



Sub Cancel_By_Patient_Letter()
'******************************************
'Text for a "Cancelled by Patient" Letter *
'******************************************
On Error Resume Next
Documents.Add Template:="C:\Cancel By Patient Letter.dot", _
NewTemplate:=False, DocumentType:=0
Call Enter_Patient_Details_Into_Letter 'separate sub
With ActiveDocument
.Bookmarks("Old_Appt_With").Range.Text = szStaff & " (" & szPosition & _
")" 'not needed so bypass
.Bookmarks("Old_Appt_Day").Range.Text = cboOldDay 'not needed so bypass
If cboOldLocation.Value = "Health Centre" Then
.Bookmarks("Old_Appt_Location").Range.Text = cboOldLocation
Else
.Bookmarks("Old_Appt_Location").Range.Text = "your home"
End If
.Bookmarks("New_Appt_With").Range.Text = szStaff & ", (" & szPosition & _
")"
.Bookmarks("New_Appt_Day").Range.Text = cboNewDay
.Bookmarks("New_Appt_Date").Range.Text = tbxNewDate
.Bookmarks("New_Appt_Time").Range.Text = tbxNewTime
If cboNewLocation.Text = "Health Centre" Then
.Bookmarks("New_Appt_Location").Range.Text = cboNewLocation
ElseIf cboNewLocation.Text = "Home Visit" Then
.Bookmarks("New_Appt_Location").Range.Text = cboNewLocation
End If
End With
Call Sign_Patient_Letter 'separate sub
End Sub


Your input would be valuable

Regards

Seamus

EDIT : reason added more underscores so fumei could see better :) Tommy

[fumei]

Please edit your post and use more underscore characters. The code window stretches wwaaaaaaaaaayyyyyyyyyyy out of sight for me. It makes it very hard for me to read.

Thanks.

I think putting your comments on their own lines should do it.

realised that virtually all letters use the same variables

BINGO! We have a bingo.

If there was a way to be able to list ALL the possible bookmarks in the sub above and have it ignore the ones that are not needed - then I could have just one sub for all letters. I am not quite following.

In my own work, the template file has ALL the bookmarks. The userform takes user input and determines what bookmarks stay, and what bookmarks are removed.

The ones that stay get values, the ones that are not needed are deleted. So I do not actually call another template, or make another new document. The originator has everything.

In fact, in your document, there does not have to be that many bookmarks really. What you have is a plethora of data.

I am not following this code: If cboOldLocation.Value = "Health Centre" Then
.Bookmarks("Old_Appt_Location").Range.Text = cboOldLocation
Else
.Bookmarks("Old_Appt_Location").Range.Text = "your home"
End IfWhy is there a True/False aspect to this? This first (if true) uses cboOldLocation, the second (if false) uses explicit text "your home".

Is not your home an item in cboOldLocation?

Why not just simply: .Bookmarks("Old_Appt_Location").Range.Text = cboOldLocationIt will put whatever is cboOldLocation - be it "Health Centre", OR "your home".

[SOS]

Firstly, sorry, I didn't realise the code window stretches to that degree.

Looks OK on my machine. Will look into fixing that,

As far as this goes:



Why is there a True/False aspect to this? This first (if true) uses cboOldLocation, the second (if false) uses explicit text "your home".


the first option in the cbo is "Health Centre" and the second is "your

home". However if option 1 is chosen it is not the text "Health Centre"

that is put into the letter but the actual name of the appointment venue.

I think that's why I went down the road I did.


Next question, if in my VBA code I have eg

.bookmarks("Admission_Date")

and the letter I'm creating doesn't have/need that then the code stops

with an error message about

"The requested member of the collection does not exist"

and I'd like to know how to avoid that.

Regards

Seamus

[SOS]

fumei,

Actually worked it out after reading your post more thoroughly.

I now have a template for each letter which includes the necessary text and all possible bookmarks. Bookmarks are then either used as a result of user input to the form or deleted by using a simple:



Dim bm as Bookmark
For Each bm in ActiveDocument.Bookmarks
bm.Delete
Next bm


and the VBA code for each letter just calls the bookmarks needed for itself.

Still looking into your reply re better error trapping.

Regards
Seamus

[fumei]

However if option 1 is chosen it is not the text "Health Centre" that is put into the letter but the actual name of the appointment venue. Huh? In the following:
If cboOldLocation.Value = "Health Centre" Then
.Bookmarks("Old_Appt_Location").Range.Text = cboOldLocation If "Health Centre" is chosen then "Health Centre" (cboOldLocation) is what is put in.


and the letter I'm creating doesn't have/need that then the code stops with an error message about "The requested member of the collection does not exist" Do a test to see if it exists.

If .Bookmarks.Exists("Admission_Date")

[SOS]

What I meant was that the value of cboOldLocation was set to "Health Centre" because I didn't want to put the real name of the place in when posting on the forum.

Otherwise I would have had to have posted




If cboOldLocation.Value = "Health Centre" Then

.Bookmarks("Old_Appt_Location").Range.Text = "xxxxxx xxxx Centre"



and I've worked out how te deal with the "not used" bookmarks.

Thanks
Seamus

[fumei]

RE: "Health Centre", ah...right, OK. But in the real world, would you not HAVE the actual name in the combobox? In which case, again, why not use it? Why use a separate text string???

Oh, and thanks for editing your post. It is much, much, more comfortable to read.

BTW: rather than suck up forum/thread space, we can move our discussion part into direct communication if you like. If you have specific questions that would be relevant to others then by all means post threads. However, the chatting/discussion could be moved off of here. This can potentially be an extended exchange, and likely to move into specifics beyond the subject of the thread.

[SOS]

Fully agree. So if I have any more discussion I'll pm you.

Seamus