2

When an array of Shapes is given to a subroutine By Reference, how can these Shapes be grouped, WITHOUT referring to them by their .name strings ?

The code below does not work:

Sub GroupShapes(ByRef ShapeArray() As Shape)
    Dim i As Long
    Dim IDs() As Variant
    
    ReDim IDs(LBound(ShapeArray) To UBound(ShapeArray))
    For i = LBound(ShapeArray) To UBound(ShapeArray)
        IDs(i) = ShapeArray(i).ID  'If .ID is changed into .Name then the objects become grouped Later, but they are being referred to by their name strings
    Next i
    
    ActiveSheet.Shapes.Range(IDs).Group
End Sub

I can make the code above work, just by changing .ID to .Name, but that is referring to the shapes by their .name strings which is exactly what I am trying to avoid.

George Robinson
  • 1,500
  • 9
  • 21
  • Could you share why you cannot just use `.Name` instead of `.ID`? – VBasic2008 Dec 21 '20 at 23:19
  • @VBasic2008 For performance reasons. Long strings take a long time to process. Much longer than one number. Excel must find the shape index in its internal structures (tree, trie) before it can access it. It pays to bypass that step and give it the index directly. The IDs were just a nice try. – George Robinson Dec 21 '20 at 23:27
  • Why do you need the shapes (objects) in an array? Could you share a code where you are using this array. – VBasic2008 Dec 21 '20 at 23:35
  • Have you tried `ActiveSheet.Shapes.Range(ShapeArray).Group`? – Variatus Dec 22 '20 at 00:06
  • I get the array of shapes from a 3rd party code. I have to interface with it, Yes I tried `ActiveSheet.Shapes.Range(ShapeArray).Group`, see https://imgur.com/In9HHl8 – George Robinson Dec 22 '20 at 00:45
  • [The documentation](https://learn.microsoft.com/en-us/office/vba/api/excel.shapes.range) suggests your code should work. Can you elaborate on "does not work" – chris neilsen Dec 22 '20 at 00:59
  • Please look at these animations to see what I mean by "does not work": https://i.imgur.com/In9HHl8.gif and https://i.imgur.com/3w6bqPO.gif – George Robinson Dec 22 '20 at 01:03
  • @GeorgeRobinson both those show trying to group an array of objects. Your posted code shows building an array of IDs from the array of objects. The docs say grouping an array of IDs should work. What I'd like to know is how your posted code fails – chris neilsen Dec 22 '20 at 01:20
  • @chris neilsen: On my end it fails with `Run-time error '1004': Application-defined or object-defined error` on the line `ActiveSheet.Shapes.Range(IDs).Group`. OP's error handler is preventing him to give you the information. – VBasic2008 Dec 22 '20 at 01:24
  • @VBasic2008 thanks, that's what I wanted to know. My next Q was going to be are we sure all the shapes referenced in the array are on the active sheet – chris neilsen Dec 22 '20 at 01:30
  • Here is the 3rd animation of the code in the Original Post. https://i.imgur.com/UZHv4j6.gif From the error message, it is evident that `.ID` does not contain the `Index` of the shape in the collection. – George Robinson Dec 22 '20 at 01:40
  • @GeorgeRobinson can you verify that all the shapes are on the active sheet? – chris neilsen Dec 22 '20 at 02:02
  • @chris neilsen: In a new workbook I added 3 shapes in `Sheet1`. I don't know how (I didn't delete any shape), but they got the IDs `2, 3, 4`. If I use `Array(1, 2, 3)` instead of `IDs` the function works. How about that. Looks like the indexes got shifted somehow. I'm using `Windows 10 64-bit` and `Office 2019 32-bit`. – VBasic2008 Dec 22 '20 at 02:05
  • @Chris Neilsen - Yes. Also, when I change `.ID` into `.Name` in the code in the Original Post then no error occurs and the shapes become grouped. – George Robinson Dec 22 '20 at 02:13
  • @George Robinson: Maybe you could test instead of `ActiveSheet.Shapes.Range(IDs).Group` to use `ActiveSheet.Shapes.Range(Array(1, 2, 3, 4, 5, 6, 7)).Group`, if these shapes are the only ones (or at least the first ones) on your worksheet. – VBasic2008 Dec 22 '20 at 02:23
  • The IDs you want will probably change as you delete shapes - so Rectangle34 might be the only shape on a sheet but be Shape(1) of the sheet. And it's the 1 you want in IDs not 34 – Tin Bum Dec 22 '20 at 02:42
  • 1
    On rereading the docs, Shapes.Range accepts an array of names or **Index**'s (not IDs). The Index and ID properties aren't the same thing. I can't see any reference to a Shape having an Index property (maybe someone could test that). – chris neilsen Dec 22 '20 at 03:52
  • Could you share your *3rd party code*, please? – VBasic2008 Dec 22 '20 at 06:54
  • It is against the rules of this forum to post closed source. – George Robinson Dec 22 '20 at 08:50
  • @Chris Neilsen The lack of an `.Index` property in a shape (...or the impossibility of obtaining it) would actually constitute an answer to this entire question ...although it does not lead to a working code. The fact that `.ID` <> `.Index` also deserves an honorable mention, for posterity, – George Robinson Dec 22 '20 at 09:02
  • @GeorgeRobinson I agree, but I can't be certain ATM because I can't test it. If you can, feel free to post a self answer. – chris neilsen Dec 22 '20 at 09:20

1 Answers1

2

As has been noted, you can create a ShapeRange by index. The difficulty is in finding the index of your shape, which isn't the same as the ID property. Additionally, your shape may already be grouped, so it won't necessarily exist at Worksheet.Shapes level

It's possible to have nested shape groups, but I believe these have to be nested from bottom-level up. In other words, I think if you try to sub-group and already grouped shape, an error will be thrown.

I may be missing something obvious, but that suggests we can group the array by finding the Worksheet.Shapes level index of a shape that either is or contains our target shape. And the index could be found by iterating those top-level shapes until the unique ID property matches. It would then be possible to create a ShapeRange on the resulting indexes.

I wonder if something like this would work:

Private Function GroupShapes(ByRef shapeArray() As Shape) As Shape
    Dim i As Long, n As Long
    Dim ws As Worksheet
    Dim sh As Shape
    Dim obj As Object
    Dim idList As Collection
    Dim id As Variant
    Dim idArray() As Long
    
    'Create the list of ids for sheet level shapes.
    Set idList = New Collection
    For i = LBound(shapeArray) To UBound(shapeArray)
        Set sh = shapeArray(i)
        Do While sh.Child
            Set sh = sh.ParentGroup
        Loop
        On Error Resume Next
        idList.Add sh.id, CStr(sh.id)
        On Error GoTo 0
    Next
    If idList.Count <= 1 Then Exit Function
    
    'Define the sheet parent.
    Set obj = shapeArray(LBound(shapeArray)).Parent
    Do Until TypeOf obj Is Worksheet
        Set obj = obj.Parent
    Loop
    Set ws = obj
    
    'Find the indexes of the shape ids.
    ReDim idArray(idList.Count - 1)
    n = 0
    For Each id In idList
        i = 1
        For Each sh In ws.Shapes
            If id = sh.id Then
                idArray(n) = i
                Exit For
            End If
            i = i + 1
        Next
        n = n + 1
    Next
    
    'Group by index.        
    Set GroupShapes = ws.Shapes.Range(idArray).Group
    
End Function

The following test seemed to work for me:

Public Sub RunMe()
    Dim shapeArray(0 To 3) As Shape
    Dim g As Shape
    
    'Create a sample array.
    'Note some of these shapes are already grouped so
    'wouldnt appear at Sheet.Shapes level.
    Set shapeArray(0) = Sheet1.Shapes("Rectangle 1")
    Set shapeArray(1) = Sheet1.Shapes("Isosceles Triangle 2")
    Set shapeArray(2) = Sheet1.Shapes("Arrow: Right 4")
    Set shapeArray(3) = Sheet1.Shapes("Oval 7")
    
    'Group the array.
    Set g = GroupShapes(shapeArray)

End Sub
Ambie
  • 4,872
  • 2
  • 12
  • 26
  • I have to accept your answer because it satisfies all the conditions in my original question, but I do not like the iterative nature of this solution as well as the associated overhead and lower performance than the `.Name` method outlined at the bottom of the OP. Because of it - I will not use it. – George Robinson Dec 22 '20 at 12:55
  • @GeorgeRobinson, yes, that's a fair comment, this method is dreadfully iterative. Incidentally, are you sure about the performance aspects of the `.Name` method? In VBA, I believe the index methods of collections are very poorly performing because VBA internally iterates each collection item incrementing a counter until the target count is reached; whereas the keys (ie the strings), are stored in hashmaps. I haven't tested it, but I suspect that using the name string would be the most efficient way of creating your `ShapeRange` to group. – Ambie Dec 22 '20 at 22:42
  • You could improve the performance of this by creating a collection mapping ID to Index. This would mean looping the shapes collection _once_ to create the collection, but avoids the nested loop – chris neilsen Dec 25 '20 at 19:57