Global variables and Userform code

[JenniferMurphy]

I am working on my first UserForm. It's for a sheet to keep track of wins and losses in a game. I have a Score button that brings up a form with Win, Lose, and Quit buttons. It's all working.

The Subs behind the Win and Lose buttons have quite a bit of code in common and they read the same data cells into variables. I hate to duplicate code. It's too easy to have them get out of sync. What's the best way to share this code?

One way I thought of is to put the common code in a separate subroutine (Setup) that each button sub can call. This would require that the variables assigned in Setup be available to the calling routines. Can I declare them as global variables outside all of the routines? If so, what's the correct way to do that?

Thanks

 

[diddi]

post the whole code and i can take a look

 

[JenniferMurphy]

OK, I simplified the code down to its basics:

' Can I put global varaibles here
' Will they retain their values as long as the parent Score button is active?



'***********************************************************
'               Win button
'
' Record a Win
'***********************************************************
Private Sub ButtonWin_Click()
Const title As String = "Win Button"
Dim GamesWon As Long, GamesLost As Long

GamesWon = Range("WinHdr").Offset(1, 0).Value   'Get games won
GamesLost = Range("LossHdr").Offset(1, 0).Value 'Get games lost

GamesWon = GamesWon + 1
Range("WinHdr").Offset(1, 0).Value = GamesWon   'Score the win

MsgBox "Win recorded", vbOKOnly, title

End Sub



'***********************************************************
'               Lose button
'
' Record a Win
'***********************************************************
Private Sub ButtonLoss_Click()
Const title As String = "Loss Button"
Dim GamesWon As Long, GamesLost As Long

GamesLost = Range("LossHdr").Offset(1, 0).Value 'Get games lost

GamesLost = GamesLost + 1
Range("LossHdr").Offset(1, 0).Value = GamesLost 'Score the loss

MsgBox "Loss recorded", vbOKOnly, title

End Sub



'***********************************************************
'               Done button
'***********************************************************
Private Sub ButtonDone_Click()
Const title As String = "Done Button"
Dim msg As String

If vbYes = MsgBox("Exit?", vbYesNo, title) Then
    MsgBox "That's all folks!!! Hurry back.", vbOKOnly, title
    Unload uScore
Else
    MsgBox "Continuing....", vbOKOnly, title
End If

End Sub

Now, where can I put global variables so that all of these routines have access to them?

 

[diddi]

hi. heres a novel approach to your problem. i try not to have global variables as they can create potential unintended (hard to debug) issues when you revisit the code and make changes down the track.
see if it does what you expected. it cuts down the code a lot but is still simple.
if you had 25 buttons all with the same code i would use a different method

'***********************************************************
'               Win button
'
' Record a Win
'***********************************************************
Private Sub ButtonWin_Click()
    UpdateScore ("Win")
End Sub

'***********************************************************
'               Lose button
'
' Record a Loss
'***********************************************************
Private Sub ButtonLoss_Click()
    UpdateScore ("Loss")
End Sub

Sub UpdateScore(Outcome As String)
    Dim GamesWon As Long, GamesLost As Long, Title As String
    Title = Outcome & " Button"
    
    GamesWon = Range("WinHdr").Offset(1, 0).Value   'Get games won
    GamesLost = Range("LossHdr").Offset(1, 0).Value 'Get games lost
    
    If Outcome = "Win" Then
        GamesWon = GamesWon + 1
    ElseIf Outcome = "Loss" Then
        GamesLost = GamesLost + 1
    End If
    
    Range("WinHdr").Offset(1, 0).Value = GamesWon   'Score the win
    Range("LossHdr").Offset(1, 0).Value = GamesLost   ' or loss
    
    MsgBox Outcome & " recorded", vbOKOnly, Title
End Sub

 

[JenniferMurphy]

hi. heres a novel approach to your problem. i try not to have global variables as they can create potential unintended (hard to debug) issues when you revisit the code and make changes down the track.

see if it does what you expected. it cuts down the code a lot but is still simple.
if you had 25 buttons all with the same code i would use a different method

That looks good. I was going to have both buttons call the same top-level sub with a parameter so it knew which one called it. You way is clearer. Thanks.

Why is the UpdateScore sub declared as just "Sub" without "Private" or something else? Does it matter?

 

[mikerickson]

You could ignore the un-affected result

Sub UpDateScore(Outcome as String)

    With Range(Outcome & "Hdr").Offset(1,0)
        .Value = .Value + 1
    End With

    MsgBox Outcome & " recorded."
End Sub

 

[diddi]

yes indeed. i sort of wanted to keep the OP code intact Mike. and many years ago you assisted me with the class version of .ocx management if you recall that could be used here (although not much point with only 2 buttons)

 

[JenniferMurphy]

OK, here's the latest version of the code. I have a few questions:

1. Should the module-level variables (GamesWon & GamesLost) be declared with "Dim" or something else?
2. Should LoadVars be declared as just "Sub", "Private Sub", or something else?
3. What about the other subs that are declared as "Private"? Should they be something else?
4. This seems to work, but is there a better way?

'***********************************************************************
' Code behind the scoring buttons
'***********************************************************************

'***********************************************************************
'                  Module Level Variables
'***********************************************************************
Dim GamesWon As Long, GamesLost As Long

'***********************************************************
'               Load Variables
'
' Loads all variables that are used by more than 1 routine
'***********************************************************
Sub LoadVars()
GamesWon = Range("WinHdr").Offset(1, 0).Value   'Get games won
GamesLost = Range("LossHdr").Offset(1, 0).Value 'Get games lost
End Sub

'***********************************************************
'               Win button
'
' Record a Win
'***********************************************************
Private Sub ButtonWin_Click()
Const title As String = "Win Button"
Call LoadVars
GamesWon = GamesWon + 1
Range("WinHdr").Offset(1, 0).Value = GamesWon
MsgBox "Win recorded", vbOKOnly, title
End Sub

'***********************************************************
'               Lose button
'
' Record a Loss
'***********************************************************
Private Sub ButtonLoss_Click()
Const title As String = "Loss Button"
Call LoadVars
GamesLost = GamesLost + 1
Range("LossHdr").Offset(1, 0).Value = GamesLost
MsgBox "Loss recorded", vbOKOnly, title
End Sub

'***********************************************************
'               Done button
'***********************************************************
Private Sub ButtonDone_Click()
Const title As String = "Done Button"
Dim msg As String
If vbYes = MsgBox("Exit?", vbYesNo, title) Then
    MsgBox "That's all folks!!! Hurry back.", vbOKOnly, title
    Unload uScore
Else
    MsgBox "Continuing....", vbOKOnly, title
End If
End Sub

Thanks

 

[diddi]

looks all fine. subs declared in a userform are private to the userform, i am fairly sure so private not required. there is no need for the Call command either. you have not saved much in terms of the duplication you where trying to remove. but if it works as yo9u intend then thats what counts

 

[JenniferMurphy]

looks all fine. subs declared in a userform are private to the userform, i am fairly sure so private not required. there is no need for the Call command either. you have not saved much in terms of the duplication you where trying to remove. but if it works as yo9u intend then thats what counts

This is a significantly simplified version. In the actual code, the LoadVars sub is considerably longer.

Thanks