Solved New to vba code (1 Viewer)

[HelpNewtoAccess]

Does this code look correct or should I be doing something different. It seems to work ok but is there a way to shorten the code.
Thanks in advance...


Option Compare Database
Option Explicit
Dim db As Database
Dim rs As Recordset
Dim URL As String
___________________________________________________________________________________________________________________________________________________________________________________
Private Sub Form_Current()
On Error GoTo Form_Current_Err

Dim varTxt1 As Variant
Dim varTxt2 As Variant
Dim varTxt3 As Variant
Dim varTxt4 As Variant
Dim vartxt5 As Variant

varTxt1 = txt1
varTxt2 = txt2
varTxt3 = txt3
varTxt4 = txt4
vartxt5 = txt5

If (IsNull(txt1)) Then
DoCmd.SetProperty "cmd1n", acPropertyVisible, "0"
Else
DoCmd.SetProperty "cmd1n", acPropertyVisible, "-1"
End If

If (IsNull(txt2)) Then
DoCmd.SetProperty "cmd2n", acPropertyVisible, "0"
Else
DoCmd.SetProperty "cmd2n", acPropertyVisible, "-1"
End If

If (IsNull(txt3)) Then
DoCmd.SetProperty "cmd3n", acPropertyVisible, "0"
Else
DoCmd.SetProperty "cmd3n", acPropertyVisible, "-1"
End If

If (IsNull(txt4)) Then
DoCmd.SetProperty "cmd4n", acPropertyVisible, "0"
Else
DoCmd.SetProperty "cmd4n", acPropertyVisible, "-1"
End If

If (IsNull(txt5)) Then
DoCmd.SetProperty "cmd5n", acPropertyVisible, "0"
Else
DoCmd.SetProperty "cmd5n", acPropertyVisible, "-1"
End If

Form_Current_Exit:
Exit Sub

Form_Current_Err:
MsgBox Error$
Resume Form_Current_Exit

End Sub
___________________________________________________________________________________________________________________________________________________________________________________
Private Sub cmdFire_Click()
On Error GoTo cmdFire_Click_Err

URL = "C:\Centre\Info\Attachments\Fire\Fire Procedure.pdf"
CreateObject("Shell.Application").Open CVar(URL)

cmdFire_Click_Exit:
Exit Sub

cmdFire_Click_Err:
MsgBox Error$
Resume cmdFire_Click_Exit

End Sub

 

[theDBguy]

Hi. Welcome to AWF!

The code can certainly be shortened, but I don't think it will make any difference in performance.

 

[Micron]

If shortening the code is the only goal, then (untested, maybe syntax not quite right) something like

Private Sub Form_Current()
On Error GoTo Form_Current_Err

Dim i As Integer

For i = 1 to 5
  Me.Controls("cmd" & i & "n").Visible = Nz(Me.Controls("txt" & i),0) = 0
Next

Form_Current_Exit:
Exit Sub

Form_Current_Err:
MsgBox "Error " & Err.Number & ": " & Err.Description
Resume Form_Current_Exit

End Sub

The original code seems a bit strange though. Why declare a bunch of variables, assign values to them then don't use the variables? Or use that syntax for making a control visible or not? I'm surprised you say it works given that you seem to be passing a string for a boolean value (0 or -1). It might be that Access is correctly interpreting your string because you declared it as a variant. No need to do that, and risky if that's why it works.

Please use code tags when post anything more than a few lines of code.

 

[pbaldy]

A couple of other thoughts. You don't use the variables so this can all be deleted:

Dim varTxt1 As Variant
Dim varTxt2 As Variant
Dim varTxt3 As Variant
Dim varTxt4 As Variant
Dim vartxt5 As Variant

varTxt1 = txt1
varTxt2 = txt2
varTxt3 = txt3
varTxt4 = txt4
vartxt5 = txt5

The more common syntax for this is:

If (IsNull(txt1)) Then
  Me.cmd1n.Visible = False
Else
  Me.cmd1n.Visible = True
End If

And some would use Boolean logic and simply have

Me.cmd1n.Visible = Not IsNull(txt1)

but some people prefer the full If/Then block, as some find the Boolean logic method confusing.

But as DBguy says those particular changes would have no appreciable effect on performance.

 

[HelpNewtoAccess]

Thanks for the help! I also have a couple other questions if someone could please help. I use a Dlookup for txt1 to txt5 in the control source on the text boxes to get values on the form (the little text box by the black note flag in picture below)…….
=DLookUp("[Notes]","qrysearchAfc","[PointID] = 148") in the control source
…………………………………………...
I also have two command buttons for each piece of equipment one is a command button note flag if the dlookup text box is null or not.(black note button one for each piece of equipment eg. cmd1n). The other command button to open a form to display all the fields associated with the piece of equipment including the note (this one is transparent over a picture of the equipment) In this command button I've placed the code like this On click...……………………….
……………………………………………..
Private Sub cmd2l_Click()
On Error GoTo cmd2l_Click_Err

DoCmd.OpenForm "frmEquipInfo", acNormal, "", "[PointID]=148", , acDialog
Me.Requery

cmd2l_Click_Exit:
Exit Sub
cmd2l_Click_Err:
MsgBox Error$
Resume cmd2l_Click_Exit
End Sub
………………………………………………….
I am wondering if this is the best way to approach this? I have to do over a thousand pieces of equipment and wondering if this is the way I should proceed. Any thoughts? I sometimes have some slight flickers when the form requeries after I close out of the Equip Information form back to the original form. Thanks again for your help...

 

[theDBguy]

Thanks for the help! I also have a couple other questions if someone could please help. I use a Dlookup for txt1 to txt5 in the control source on the text boxes to get values on the form (the little text box by the black note flag in picture below)…….
=DLookUp("[Notes]","qrysearchAfc","[PointID] = 148") in the control source
…………………………………………...
I also have two command buttons for each piece of equipment one is a command button note flag if the dlookup text box is null or not.(black note button one for each piece of equipment eg. cmd1n). The other command button to open a form to display all the fields associated with the piece of equipment including the note (this one is transparent over a picture of the equipment) In this command button I've placed the code like this On click...……………………….
……………………………………………..
Private Sub cmd2l_Click()
On Error GoTo cmd2l_Click_Err

DoCmd.OpenForm "frmEquipInfo", acNormal, "", "[PointID]=148", , acDialog
Me.Requery

cmd2l_Click_Exit:
Exit Sub
cmd2l_Click_Err:
MsgBox Error$
Resume cmd2l_Click_Exit
End Sub
………………………………………………….
I am wondering if this is the best way to approach this? I have to do over a thousand pieces of equipment and wondering if this is the way I should proceed. Any thoughts? I sometimes have some slight flickers when the form requeries after I close out of the Equip Information form back to the original form. Thanks again for your help...
View attachment 82340

Hi. Sounds like you're using an unbound form for this, is that correct? If so, I am not sure what would be the purpose of using Me.Requery. I think screen flicker is just a byproduct of refreshing the data in the form. To minimize the amount of code you have to write, you could try converting your sub into a standard function and simply call it from your buttons. You might be able to use the Screen object, although you may need to be careful when using it.

 

[Micron]

I'm trying to think of a reason why the form just can't be bound to a query or table. Why so many DLookups, which are just a sort of a 'compartmentalized' query anyway?
And yes, requery on an unbound form is a bit like a sledgehammer on a tack. Should do the job, but a Recalc will probably suffice.

 

[pbaldy]

Regarding the OpenForm, you can refer to a form value rather than hard coding the value. That would let you have a single button.

Open a second form to the record

 

[HelpNewtoAccess]

Both forms are bound to a query that contains the note. Some images have 20 or more pieces of equipment so just trying to sort this out before I really get into this. Thanks

 

[HelpNewtoAccess]

Hi Micron,
Is this acceptable because It was backwards or should I write the code a different way...Thanks this will save me a ton of time.
Me.Controls("cmd" & i & "n").Visible = false = Nz(Me.Controls("txt" & i),0) = 0

 

[Micron]

It looks like it would work but I probably would not have thought of that. I think generally most of us would have written it as
Me.Controls("cmd" & i & "n").Visible = Not Nz(Me.Controls("txt" & i),0) = 0
One less = might be the only difference.

One thing I'm not grasping is why all the DLookups (lots of them, yes?) if the form is bound to a query. Wondering also if your controls with these values are locked, or the form recordsource is not editable so that I can't delete a value and make a button disappear and not be able to get it back.

 

[HelpNewtoAccess]

Thanks for the code. The text box is positioned behind the note flag button so the note can’t be changed without going to the other form. I placed it in the picture attached with the text box showing but it is usually behind the note flag. With the Dlookups yes there is a lot of them how would you do this different? Suggestions I’m open to anything. Thanks

 

[The_Doc_Man]

With regard to this statement:

Me.Controls("cmd" & i & "n").Visible = false = Nz(Me.Controls("txt" & i),0) = 0

That red equals sign shouldn't work because you are attempting to assign an expression's value to an intrinsic constant, First, I don't recall that VBA allows that kind of transitive multi-expression assignment. VB might, I would have to look for that one. But languages like C or C++ or C# allow the transitive expressions. VBA? Not so much. And I think even C-related would barf if you tried to assign a value to an intrinsic constant.

 

[Isaac]

I use

If [condition] then
  -result
End if

format.
Yes it's kind of slick to do it all on one line, but to me readability is paramount. For me personally, it's slightly easier to debug too.

 

[cheekybuddha]

Hi,

Along with the others, I can't quote see why you're not using a bound form.

However, if you have your reasons, you should be able to reduce flicker by not setting a property to whatever it is already.

Adapting Micron's code from above, you can try:

Private Sub Form_Current()
On Error GoTo Form_Current_Err

Dim i As Integer

With Me
  For i = 1 to 5
    If Not (.Controls("cmd" & i & "n").Visible = Not Nz(.Controls("txt" & i),0) = 0) Then
      .Controls("cmd" & i & "n").Visible = Not Nz(.Controls("txt" & i),0) = 0
    End If
  Next
End With

Form_Current_Exit:
Exit Sub

Form_Current_Err:
MsgBox "Error " & Err.Number & ": " & Err.Description
Resume Form_Current_Exit

End Sub

hth,

d

 

[Micron]

That red equals sign shouldn't work because you are attempting to assign an expression's value to an intrinsic constant, First, I don't recall that VBA allows that kind of transitive multi-expression assignment. VB might, I would have to look for that one. But languages like C or C++ or C# allow the transitive expressions. VBA? Not so much. And I think even C-related would barf if you tried to assign a value to an intrinsic constant.
Me.Controls("cmd" & i & "n").Visible = false = Nz(Me.Controls("txt" & i),0) = 0

Nice in theory but it does work. Probably is being evaluated as False = False or as False = True

 

[HelpNewtoAccess]

What should I do instead of the Dlookups suggestions?

 

[Micron]

It's been suggested about 3 times - a bound form. As noted, if you cannot bind the form for some reason then I suppose you continue with your current approach.

 

[HelpNewtoAccess]

The form is bound to a query like I said above but how would I approach the code? (If there are numerous note flags each tied to a different specific record). I'm probably missing something really easy but if someone could help it would be much appreciated.

 

[Micron]

If the controls you're doing the DLookup for are getting data from the same source as what the form is bound to, then just set the controlsource property of those controls to the proper field in that source. If that isn't what's going on, then I don't know what you need to do. I've read this thread a couple of times for hints on that and didn't see where you've ever said exactly what's going on. If you want to follow that deeper, then no problem, but either more info about the design is required, or better yet, post a db copy that you have compacted then zipped, along with some instructions on what to do and what to look for.