4
\$\begingroup\$

I have the following code I'm using to show/hide multiple shape groups in Excel 2016 using VBA and macros:

   Sub Pic_1_SA_click()
   ActiveSheet.Shapes("Group 23").Visible = True
   ActiveSheet.Shapes("Group 71").Visible = False
   ActiveSheet.Shapes("Group 19").Visible = False
   ActiveSheet.Shapes("Group 20").Visible = False
   End Sub

   Sub Pic_1_SB_click()
   ActiveSheet.Shapes("Group 23").Visible = False
   ActiveSheet.Shapes("Group 71").Visible = True
   ActiveSheet.Shapes("Group 19").Visible = False
   ActiveSheet.Shapes("Group 20").Visible = False
   End Sub

   Sub Pic_2_SA_click()
   ActiveSheet.Shapes("Group 23").Visible = False
   ActiveSheet.Shapes("Group 71").Visible = False
   ActiveSheet.Shapes("Group 19").Visible = True
   ActiveSheet.Shapes("Group 20").Visible = False
   End Sub

   Sub Pic_2_SB_click()
   ActiveSheet.Shapes("Group 23").Visible = False
   ActiveSheet.Shapes("Group 71").Visible = False
   ActiveSheet.Shapes("Group 19").Visible = False
   ActiveSheet.Shapes("Group 20").Visible = True
   End Sub

Now the above is somewhat tedious if I plan to add more buttons. Is there any way to do this more efficiently?

\$\endgroup\$

closed as off-topic by Graipher, Stephen Rauch, t3chb0t, Mathias Ettinger, Daniel Jul 5 '18 at 10:49

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Graipher, Stephen Rauch, t3chb0t, Daniel
If this question can be reworded to fit the rules in the help center, please edit the question.

3
\$\begingroup\$

First off, I'll take note of my assumptions:

  • You have four distinct groups of shapes
  • On each click event you hide all but one of them
  • The order in which the event procedures are executed is not known/assumed to be random

To start refactoring, I'd save the group names in module constants.

Private Const NameGroupA As String = "Group 23"
Private Const NameGroupB As String = "Group 71"
Private Const NameGroupC As String = "Group 19"
Private Const NameGroupD As String = "Group 20"

This way, should you decide to rename them or something changes, it only needs to be changed in one place.

With the assumed randomness in the event execution order, there is no point in trying to find a specific pattern that could be encoded to optimize things. So the next thing we can to is define a subroutine that hides all groups:

Private Sub HideAllGroups(ContainingSheet as Worksheet)
    With ContainingSheet.Shapes
        .Item(NameGroupA).Visible = False
        .Item(NameGroupB).Visible = False
        .Item(NameGroupC).Visible = False
        .Item(NameGroupD).Visible = False
    End With
End Sub

Replacing the ActiveSheet by a parameter makes the procedure (somewhat) reusable. (Not very useful in this specific case, but in general a good practice.)

This leads to you only needing to specify one group to be shown in each event procedure:

Sub Pic_1_SA_click()
    HideAllGroups ActiveSheet
    ActiveSheet.Shapes(NameGroupA).Visible = True
End Sub

Now if you expect you will need several more similar procedures, then you could even go further and define a subroutine that takes a worksheet and a GroupName to hide all but the specified group:

Private Sub HideAllButSpecifiedGroup(ContainingSheet as Worksheet, ByVal GroupName as String)
    HideAllGroups ContainingSheet 
    ContainingSheet.Shapes(GroupName).Visible = True
End Sub

which then simplifies the event procedures like this:

Sub Pic_1_SA_click()
    HideAllButOneGroup ActiveSheet, NameGroupA
End Sub

This is as far as I'm able to simplify the process. You might want to have some parameter checking and error handling in place, depending on where and how the code is going to be used.

\$\endgroup\$
  • \$\begingroup\$ Thanks Inarion! I've taken note of your method as well. I'll compare both methods when the number of items grows larger. \$\endgroup\$ – Matlas Jul 4 '18 at 8:50
5
\$\begingroup\$

The simple answer is to use pass an array of names to the Shapes.Range, hide all 4 Groups, and then make the one Group visible.

ActiveSheet.Shapes.Range(Array("Group 19", "Group 20", "Group 23", "Group 71")).Visible = msoTrue
ActiveSheet.Shapes("Group 20").Visible = msoFalse

Reference: MSDN - Shapes.Range Property(Excel)

Considering that all Group default names start with "Group ", you could further simplify it by creating a function that will return the ShapeRange based of the Group number.


Public Function getGroupRange(ws As Worksheet, ParamArray GroupNames() As Variant) As ShapeRange
    Dim n As Long
    For n = 0 To UBound(GroupNames)
        If IsNumeric(GroupNames(n)) Then GroupNames(n) = "Group " & GroupNames(n)
    Next
    Set getGroupRange = ws.Shapes.Range(GroupNames)

End Function

Sub Pic_1_SA_click()
    getGroupRange(ActiveSheet, 19, 20, 23, 71).Visible = msoFalse
    getGroupRange(ActiveSheet, 23).Visible = msoTrue
End Sub

Sub Pic_1_SB_click()
    getGroupRange(ActiveSheet, 19, 20, 23, 71).Visible = msoFalse
    getGroupRange(ActiveSheet, 71).Visible = msoTrue
End Sub

Sub Pic_2_SA_click()
    getGroupRange(ActiveSheet, 19, 20, 23, 71).Visible = msoFalse
    getGroupRange(ActiveSheet, 19).Visible = msoTrue
End Sub

Sub Pic_2_SB_click()
    getGroupRange(ActiveSheet, 19, 20, 23, 71).Visible = msoFalse
    getGroupRange(ActiveSheet, 20).Visible = msoTrue
End Sub
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.