Creating a loop to replace multiple nested if statements when dealing with groups of objects in Excel

Question

I'm not sure how to optimize the code attached below into some form of loop and am hoping someone would be able to illustrate how best to tackle this.

Basically, I've inherited a spreadsheet with several VBA modules recorded from the macro recorder and/or written by someone inexperienced with VBA, and it's very slow to run. I've been going through and reducing a lot of redundant 'nested if' type sections into for loops in an attempt to optimize and speed things up, however I'm also very inexperienced and really not a coder myself as you can probably guess!

If Range("Link1").Value = "" Then
    Application.CutCopyMode = False

    GoTo Finale:
Else

If Range("Link2").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1")).Select
    Selection.Copy
Else

If Range("Link3").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1", "Group2")).Select
    Selection.Copy
Else

If Range("Link4").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3")).Select
    Selection.Copy
Else

If Range("Link5").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4")).Select
    Selection.Copy
Else

If Range("Link6").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4", "Group5")).Select
    Selection.Copy
Else

If Range("Link7").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4", "Group5", "Group6")).Select
    Selection.Copy
Else

If Range("Link8").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4", "Group5", "Group6", "Group7")).Select
    Selection.Copy
Else

    ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4", "Group5", "Group6", "Group7", "Group8")).Select
    Selection.Copy

End If

End If

End If

End If

End If

End If

End If

End If

The code deals with copying 8 groups of 'things' (in this case, each containing text boxes and a graphic) and checks if a link has been populated, copying the previous groups when it finds an unpopulated link. The idea here is therefore that only populated groups are copied.

A second question regarding all this is that when you have multiple if statements like this, is it actually meaningfully faster or more optimal to reduce such things into loops, or should I be looking elsewhere to further optimize the spreadsheet? Turning long passages of recursive code into minimal loops certainly feels good(!), but I don't know if that's really what actually needs to be done to improve speed and stability or if it actually has little meaningful impact.

From a quick glance, you are **way** better off using elseif statements instead of using nested if statements — Tim Stack, Jun 12 '19 at 08:31
And read [How to avoid using Select in Excel VBA](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba) — Tim Stack, Jun 12 '19 at 08:32
@TimStack exactely on point. I was looking to link to that article. — rohrl77, Jun 12 '19 at 08:33

score 0 · Accepted Answer · answered Jun 12 '19 at 08:32

0

It's hard to tell what your code is doing from what you are providing, but there are several ways to make at least this part more efficient.

The first is to use the ElseIf construct. This prevents you from having to do many nested if statements.

Second, you want to avoid using Select whenever possible.

Here is a sample of what you can do to refactor your code:

If Range("Link2").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1")).Copy
ElseIf Range("Link3").Value = "" Then
    ActiveSheet.Shapes.Range(Array("Group1", "Group2")).Copy
Elseif ...

answered Jun 12 '19 at 08:32

rohrl77

3,277
11
47
73

The absence of a specified workbook and -sheet, and the reference to `ActiveSheet` also is an open door for errors – Tim Stack Jun 12 '19 at 08:39
Agreed... but trying to take it a step at a time since the OP said he is new to coding – rohrl77 Jun 12 '19 at 08:40
Thanks very much @rohrl77 - As I just described elsewhere, for some reason when I remove 'Select', it results in a "Run-time error 438: Object doesn't support this property or method" - but both points are still really useful advice and much appreciated- many thanks! :) – Turtle Jun 12 '19 at 08:55

Dmitrij Holkin · Answer 2 · 2019-06-12T10:22:12.040

0

or just use Select Case

Application.ScreenUpdating = False
Select Case Range("Links").Value
        Case "1": ActiveSheet.Shapes.Range(Array("Group1")).Group.Copy
        Case "2": ActiveSheet.Shapes.Range(Array("Group1", "Group2")).Group.Copy
        Case "3": ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3")).Group.Copy
        Case "4": ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4")).Group.Copy
        Case "5": ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4", "Group5")).Group.Copy
        Case "6": ActiveSheet.Shapes.Range(Array("Group1", "Group2", "Group3", "Group4", "Group5", "Group6")).Group.Copy
        Case Else: Application.CutCopyMode = False: GoTo Finale
    End Select
Application.ScreenUpdating = True

where

Range("Links").Value is only one cell where you type prefered number

edited Jun 12 '19 at 10:22

answered Jun 12 '19 at 08:41

Dmitrij Holkin

1,995
3
39
86

Try to help OP avoid using `Select` – Tim Stack Jun 12 '19 at 08:45
But why you think OP want to avoid it? Maybe it want to select Shapes to see what is copying, and I make first Case to avoid it )) – Dmitrij Holkin Jun 12 '19 at 08:46
The responses are coming in so quick, thanks so much for all the information and the ideas! @TimStack I don't know if this relates to some other bad practices, but removing 'select' actually breaks this for me, I get a "Run-time error '438' - Object doesn't support this property or method". I'll read through the link on avoiding select and incorporate it elsewhere when I can thanks. – Turtle Jun 12 '19 at 08:52
@DmitrijHolkin Thanks very much, I'll give that a try and report back- I am enough of a newbie that I don't understand cases yet, but I'll have to read up :) – Turtle Jun 12 '19 at 08:52
@DmitrijHolkin you don't need to use the dreaded `Select` to see that.. – Tim Stack Jun 12 '19 at 09:19
@Turtle you have to replace the `Select` function, not remove the word. Using `Select` is bad practice and only necessary in a very rare set of cases, so try to avoid using it. Again, read [this thread](https://stackoverflow.com/a/10718179/10540017) – Tim Stack Jun 12 '19 at 09:20
@TimStack Thanks Tim, I was literally just reading that thread when you responded :) Unfortunately for me it's really not working; I misphrased what I was trying to say previously but replacing 'select' with 'copy' doesn't work in this case for me. So for example [ ActiveSheet.Shapes.Range(Array("Group1")).Copy ] throws up an error. For some reason, having [ ActiveSheet.Shapes.Range(Array("Group1")).Select followed by Selection.Copy ] works. Any ideas why, is this likely to be tied to some other bad practice in the code? – Turtle Jun 12 '19 at 09:28
[This answer](https://stackoverflow.com/a/42359976/10540017) contains two alternatives for using `Select`, see if this helps you – Tim Stack Jun 12 '19 at 10:21
@DmitrijHolkin Just to follow up that I really appreciate your suggestion, however the need to type in a number didn't really fit in the given situation. It's super useful as a concrete example of how case can be used though- thanks very much :) – Turtle Jun 13 '19 at 13:44
@TimStack Thanks for the link and the further ideas. Unfortunately I couldn't get it to work for me, the grouping looked promising however I was grouping things that had already been grouped I guess- I still received an error. I need to make some progress with more important functionality but will definitely revisit this when I have time. Thanks again :) – Turtle Jun 13 '19 at 13:47
Select case is useful in any situation, as you can do everithing you want for example check for boolean `Select Case True` and then `case Range("Links").Value = "" : ActiveSheet.Shapes.Range(Array("Group1")).Group.Copy` – Dmitrij Holkin Jun 13 '19 at 15:42

Creating a loop to replace multiple nested if statements when dealing with groups of objects in Excel

2 Answers2