I need help with this "simple" form

  • Thread starter Thread starter Tom Hanley
  • Start date Start date
T

Tom Hanley

Download the zip file from www.lanctully.com/Gen_desc.zip

I am puzzled by this form! You select multiple check boxes. Press command
button and nothing happens the first time. Close the form and reopen it. try
it again and it builds a text string puts the text in the textbox. It isn't
what I want it to put there... I want to capture the label caption text
instead. Funny thing is that is I put a stop in the code and test the
variable, it gives me the correct caption info in the immediate window. I
realize that this is a very basic form and I am missing something obvious
but I have been staring at it for two days and need a new pair of eyes to
help me out.

TIA

Tom Hanley
 
I can't reproduce your first problem with the button not working the first
time you open the form.

This code does what you want:

Private Sub Gen_desc_Click()

Dim i As Integer
Dim str As String

For i = 0 To 21
If Me("Check" & i) Then
str = str & Me("Label" & i).Caption & ", "
End If
Next

str = Left(str, Len(str) - 2)
Me.Work_req_desc = str

End Sub
 
NB - There is bug in my code if nothing is selected. Use this instead:

Dim i As Integer
Dim str As String
Dim intLen As Integer

For i = 0 To 21
If Me("Check" & i) Then
str = str & Me("Label" & i).Caption & ", "
End If
Next

intLen = Len(str)

If intLen Then
str = Left(str, intLen - 2)
End If

Me.Work_req_desc = str
 
Tom,

One more point, then I'll shut up!

I don't know what you're doing with this form, but it occurs to me that it
would probably be better to put your list of options in a table and then set
up a form where you could select the options from a list box. That way, if
the options change you don't have to redesign the form.

Andrew
 
Thx pal... that does what I wanted. I didn't choose to make a list because I
wanted to make it easy to see what is checked at a glance. this form is
really just to develope the code. I haven't decided how I will present the
options to the users yet. I still don't see why my code was wrong but thanks
again for your time and effort.
 
Tom Hanley said:
I still don't see why my code was wrong but thanks
again for your time and effort.
Click to expand...

Tom,

Your code is a good bit more complicated than it needs to be and this always
makes it harder to debug. However the reason it does not work is this line:

wkdesc = "Me![Label" + Trim(str(counter)) + "].caption"

This will create a literal string, eg if Trim(str(counter)) returns 1 the
the value of wkdesc will be:

"Me!Label1.caption"

NOT whatever is the value of the caption of Label1. If you change the line
to

wkdesc = Me(Label + Trim(str(counter)).Caption

then it will work. Even better change it to:

wkdesc = Me("Label" & counter).Caption

as you don't need to convert "counter" to a string first.

There are at least three different forms of syntax that you can use to refer
to the caption of Label1:

Me.Label1.Caption
Me!Label1.Caption or
Me("Label1").Caption

In this situation you have to use the last option as this is the only one
where "Label1" is a string.

Some other points about your code:

- You use the + symbol to concatenate strings. This is why you've had to
convert your integer variables to strings before you can concatenate them.
If you use the & symbol then you don't need to do this conversion, so it's
better to just use & for concatenation.

- Dim wkreq, wkdesc As String will not create two string variables - wkreq
will be a variant. You should use a separate line for each variable.

- I never use IIF in VBA code - you can always use If .. then .. else. The
problem with IIF is that it will always evaluate both arguments so is
slower. Also it can create an error by evaluating the argument that you
don't need.
 
I really appreciate your detailed explanation. I understand it a lot better
now. It is great to have folks like you help out the rest of us especially
when the questions to be answered are usually very difficult to convey.
Thanks again!

Andrew Smith said:
Tom Hanley said:
I still don't see why my code was wrong but thanks
again for your time and effort.
Click to expand...

Tom,

Your code is a good bit more complicated than it needs to be and this always
makes it harder to debug. However the reason it does not work is this line:

wkdesc = "Me![Label" + Trim(str(counter)) + "].caption"

This will create a literal string, eg if Trim(str(counter)) returns 1 the
the value of wkdesc will be:

"Me!Label1.caption"

NOT whatever is the value of the caption of Label1. If you change the line
to

wkdesc = Me(Label + Trim(str(counter)).Caption

then it will work. Even better change it to:

wkdesc = Me("Label" & counter).Caption

as you don't need to convert "counter" to a string first.

There are at least three different forms of syntax that you can use to refer
to the caption of Label1:

Me.Label1.Caption
Me!Label1.Caption or
Me("Label1").Caption

In this situation you have to use the last option as this is the only one
where "Label1" is a string.

Some other points about your code:

- You use the + symbol to concatenate strings. This is why you've had to
convert your integer variables to strings before you can concatenate them.
If you use the & symbol then you don't need to do this conversion, so it's
better to just use & for concatenation.

- Dim wkreq, wkdesc As String will not create two string variables - wkreq
will be a variant. You should use a separate line for each variable.

- I never use IIF in VBA code - you can always use If .. then .. else. The
problem with IIF is that it will always evaluate both arguments so is
slower. Also it can create an error by evaluating the argument that you
don't need.
Click to expand...
 
Back
Top