Blacksmith
Blacksmith

Reputation: 37

Looking for a better way to set properties for form controls

I'm using the code below in many forms and several applications:

Private Sub EnableEdit(strFieldname As String, Optional bUseRed As Boolean = False)
  Me.Controls(strFieldname).Enabled = True
  Me.Controls(strFieldname).Locked = False
  If Not Me.Controls(strFieldname).ControlType = acCheckBox Then
    Me.Controls(strFieldname).BackStyle = 1
    If bUseRed Then
      Me.Controls(strFieldname).ForeColor = vbRed
    Else
      Me.Controls(strFieldname).ForeColor = vbBlack
    End If
  End If
End Sub

' and the calls e.g.
  EnableEdit ("arHerstellerArtikelNr")
  EnableEdit ("arEAN")
  EnableEdit ("arArtikelNrIxxtraNice")
  EnableEdit ("arSort")

A recent change showed me, that it would be much better to have the code NOT duplicated in each Class Object. First my problem doing so was, that the code contains 'Me.Controls(strFieldname)' to address the properties I want to manipulate. For this reason it had to be in the Class Object itself --> requiring duplication.

So I rewrote my code. The design goal was: a way to call 'EnableEdit' from the events of several forms, but supplying the code for this not in the forms module itself, but in a module or class module that is supplied just once for the application. I think the way access works, I need to pass an additional parameter to distinguish the forms. However if the name of the form needs to be passed as an additional parameter (e.g. EnableEdit ("frmInventoryUpdate", "arSort")) the code is less readable and more error prone. So I came up with the idea to pass the name as an invariant: 'EnableEdit (Me.FormName, "arSort")'. This led me to the following solution:

Public Sub EnableEdit2(strFormName As String, strFieldname As String, Optional bUseRed As Boolean = False)
Dim frmCurrentForm As Form
  Set frmCurrentForm = Forms(strFormName)
  frmCurrentForm.Controls(strFieldname).Enabled = True
  frmCurrentForm.Controls(strFieldname).Enabled = True
  frmCurrentForm.Controls(strFieldname).Locked = False
  If Not frmCurrentForm.Controls(strFieldname).ControlType = acCheckBox Then
    frmCurrentForm.Controls(strFieldname).BackStyle = 1
    If bUseRed Then
      frmCurrentForm.Controls(strFieldname).ForeColor = vbRed
    Else
      frmCurrentForm.Controls(strFieldname).ForeColor = vbBlack
    End If
  End If
End Sub

' and the calls e.g.
  Call EnableEdit2(Me.FormName, "arHerstellerArtikelNr")
  Call EnableEdit2(Me.FormName, "arEAN")
  Call EnableEdit2(Me.FormName, "arArtikelNrIxxtraNice")
  Call EnableEdit2(Me.FormName, "arSort")

The solution is acceptable for me, however I wonder if there is any way to suppress even the 'Me.FormName' in the call and thus making the calls nicer?

Upvotes: 0

Views: 166

Answers (1)

Erik A
Erik A

Reputation: 32682

Never pass the form name, always pass the form:

Public Sub EnableEdit2(frmCurrentForm As Form, strFieldname As String, Optional bUseRed As Boolean = False)
  frmCurrentForm.Controls(strFieldname).Enabled = True
  frmCurrentForm.Controls(strFieldname).Enabled = True
  frmCurrentForm.Controls(strFieldname).Locked = False
  If Not frmCurrentForm.Controls(strFieldname).ControlType = acCheckBox Then
    frmCurrentForm.Controls(strFieldname).BackStyle = 1
    If bUseRed Then
      frmCurrentForm.Controls(strFieldname).ForeColor = vbRed
    Else
      frmCurrentForm.Controls(strFieldname).ForeColor = vbBlack
    End If
  End If
End Sub

'Call it:
EnableEdit2 Me, "arHerstellerArtikelNr" 
'Never use Call, that's a relic of very old code and not needed in modern code

Passing the form name causes additional overhead, goes wrong when your form is a subform, and goes wrong when you want to allow multiple instances of the same form (that all have the same name of course).

You always want to pass the form directly, both because it avoids these problems and because it's more concise.

For a more thorough rework, I'd avoid passing the field name, and would use the tag property instead to determine which fields need this enabling/disabling, given that fields don't need to be disabled or enabled independently.

Upvotes: 1

Related Questions