Working with group of checkboxes in MS Excel

I have the following checkbox as depicted by the image below:

The checkboxes are in 3 groups. Confidentiality, Integrity ad Availability. When the first in is clicked, the Confidentiality turns Red for High, Orange for Medium and Green for Low.

This is a lot of work to be done as you need to create a subroutine for each checkbox control. Below is a sample code for 2 checkboxes in the first line in the image above.

Private Sub CheckBox21_Change()

With Me.CheckBox21
If .Value Then ClearGroup .GroupName, .Name
End With

End Sub

Private Sub CheckBox22_Change()

With Me.CheckBox22
If .Value Then ClearGroup .GroupName, .Name
End With

End Sub

Private Sub ClearGroup(sGroup As String, sName As String)
    Dim ole As OLEObject

        For Each ole In Me.OLEObjects
            If TypeName(ole.Object) = "CheckBox" Then
                If ole.Object.GroupName = sGroup And ole.Name <> sName Then
                    ole.Object.Value = False
                End If
            End If
        Next ole
End Sub

Private Sub CheckBox21_Click()
    If CheckBox21.Value = True Then
    Range("i6", "m6").Value = "High"
    Range("r3").Value = "H"
    Range("i6", "m6").Interior.Color = RGB(217, 0, 0)
    Range("r3").Interior.Color = RGB(217, 0, 0)

    Else
    CheckBox21.Value = False
    Range("i6", "m6").Value = "Low"
    Range("r3").Value = "L"
    Range("i6", "m6").Interior.Color = RGB(153, 204, 0)
    Range("r3").Interior.Color = RGB(153, 204, 0)
    End If

End Sub
Private Sub CheckBox22_Click()
    If CheckBox22.Value = True Then
    Range("i6", "m6").Value = "Medium"
    Range("r3").Value = "M"
    Range("i6", "m6").Interior.Color = RGB(255, 204, 0)
    Range("r3").Interior.Color = RGB(255, 204, 0)

    Else
    CheckBox22.Value = False
    Range("i6", "m6").Value = "Low"
    Range("r3").Value = "L"
    Range("i6", "m6").Interior.Color = RGB(153, 204, 0)
    Range("r3").Interior.Color = RGB(153, 204, 0)
    End If
End Sub

Is there any other way I can write the VBA c

Let's address your question first, then we'll talk about some other things that can be improved.


This is a lot of work to be done as you need to create a subroutine for each checkbox control.

Well, yes and no. You do need to create an event routine for each control, but there's no reason to repeat all of that code over and over inside each of them. Extract the logic into a paramaterized method. Then, your event procedures will look something like this.

Private Sub CheckBox21_Click()
    HandleCheckBoxClick() 'we'll get to the parameters later
End Sub

Private Sub CheckBox22_Click()
    HandleCheckBoxClick()
End Sub

Which isn't really so much work, is it?

However, to make this work cleanly, we'll need to define a simple data structre and populate it with appropriate values for high, medium, and low. I've used a Type here, but it might be worth creating a full blown class for down the road.

Private Type TStatus
    Name As String
    Abbreviation As String
    Color As Long
End Type

You'll want to store your three different statuses at the module level and create them at an appropriate time. Perhaps on the sheet activate event. Otherwise, you'll need to create them on the fly each time the procedure is run. I'm not sure which way I'd go, but the important part is abstracting the data right now.

Private high As TStatus
Private medium As TStatus
Private low As TStatus

Private Sub InitStatusVariables()
    high.Name = "High"
    high.Abbreviation = "H"
    high.Color = RGB(217, 0, 0) 'Red

    medium.Name = "Medium"
    medium.Abbreviation = "M"
    medium.Color = RGB(255, 204, 0) 'Orange

    low.Name = "Low"
    low.Abbreviation = "L"
    low.Color = RGB(153, 204, 0) ' Green
End Sub

So now, we can go ahead and write that HandleCheckBoxClick method. We'll pass in the actual checkbox control along with our predefined true state and false state.

Private Sub HandleCheckBoxClick(cntrl As CheckBox, trueState As TStatus, falseState As TStatus)
    If cntrl.Value = True Then
        Range("i6", "m6").Value = trueState.Name
        Range("r3").Value = trueState.Abbreviation
        Range("i6", "m6").Interior.Color = trueState.Color
        Range("r3").Interior.Color = trueState.Color

    Else
        cntrl.Value = False
        Range("i6", "m6").Value = falseState.Name
        Range("r3").Value = falseState
        Range("i6", "m6").Interior.Color = falseState.Color
        Range("r3").Interior.Color = falseState.Color
    End If
End Sub

Awesome, but now the duplication inside of the method has become really obvious, but thankfully, easier to deal with. So let's refactor again.

Private Sub HandleCheckBoxClick(cntrl As CheckBox, trueState As TStatus, falseState As TStatus)

    Dim state As TStatus
    If cntrl.Value = True Then
        state = trueState
    Else
        cntrl.Value = False
        state = falseState
    End If

    Range("i6", "m6").Value = state.Name
    Range("r3").Value = state.Abbreviation
    Range("i6", "m6").Interior.Color = state.Color
    Range("r3").Interior.Color = state.Color

End Sub

Note that in your full code, you will likely need to also pass the appropriate ranges into this method as well. Also, I'm not sure if you need this line of code, so I've left it's equivalent in the above method.

Else
CheckBox22.Value = False

Why set the value of the checkbox to False if it's not True? Shouldn't it already be false? Or can the value of a checkbox be "null"? I can't remember, but it's probably worth a comment if you're doing this to make sure the value is false instead of Empty.

This feels like an abuse of the With statement to me.

With Me.CheckBox21
If .Value Then ClearGroup .GroupName, .Name
End With

Why the With statement here? I don't see a real need for it. Maybe it's personal preference, but I think this version of it is nicer.

If Me.CheckBox21.Value = True Then 
    ClearGroup Me.Checkbox21.GroupName, Me.CheckBox21.Name
End If

Even better, instead of passing strings to the ClearGroup method, pass the whole CheckBox control.

If Me.CheckBox21.Value = True Then ClearGroup Me.CheckBox21

There ya go. One line, nice and neat. It would also remove the need to check the type of the control within the method, because you already know it's a checkbox. It has to be, or you get a type mismatch error.

Your indentation is inconsistent. You do very well in some places, but very poorly in others.

Good!

For Each ole In Me.OLEObjects
        If TypeName(ole.Object) = "CheckBox" Then
            If ole.Object.GroupName = sGroup And ole.Name <> sName Then
                ole.Object.Value = False
            End If
        End If
    Next ole

Bad!

```
Private Sub CheckBox21_Click()
If CheckBox21.Value = True Then
Range("i6", "m6").Value = "High"
Range("r3").Value = "H"
Range("i6", "m6").Interior.Color = RGB(217, 0, 0)
Range("r3").Interior.Color = RGB(217, 0, 0)

StackExchange Code Review Q#87165, answer score: 3