2

Original question

I have a sub with a 24-deep nested IF statement

l=2
While l <= lmax                'lmax = 15000
   If condition1 Then
      If condition2a or condition2b Then
         ...
            If condition24 then
               ReDim Preserve propositions(UBound(propositions) + 1)
               propositions(UBound(propositions)) = l

Since this sub is called 250 times, the IF statement get called 250 * 15000, thus performance is a big issue. (The macro run in about 23 seconds.)

When I write If condition2a or condition2b Then, does VBA check condition2b if condition2a is true ? (Ie, should ordrer a and b so that a is less often true that b ?)

PS : Of course, condition 1 vs 2 are already ordered.

Short answer

As stated by @iDevlop, the short answer seems to be that VBA doesn't allow "short-circuit evaluation" (See this post)

Solution to my performance problem

My problem was VBA reading/accessing data from the sheet (rather than VBA computing the IF statement.).

The solution was loading data in a 2D-array. This single modification make my Sub run more than 10 times quicker (less than 2s vs 23s).

Original code

Here is a shorter (17-deep) version of my statement :

With Sheets("Sheet1")
        lmax = .Cells(100000, 1).End(xlUp).Row    'Usually 14000
        l = 2
        While l <= lmax
            If boolean_ignore_param1 Or Left(.Cells(l, 1).Formula, Len(param1)) = param1 Then
                If boolean_ignore_param2 Or Left(.Cells(l, 2).Formula, Len(param2)) = param2Then
                    If (param_boolean_A And .Range("AF" & l).Formula = "Yes") Or (param_boolean_B And .Range("Ag" & l).Formula = "Yes") Then
                        If (.Cells(l, 6).Formula = "" Or .Cells(l, 6).Value - marge <= param3 Or param3= 0) Then
                        If (.Cells(l, 7).Formula = "" Or .Cells(l, 7).Value + marge >= param3 Or param3 = 0) Then
                        If (.Cells(l, 8).Formula = "" Or .Cells(l, 8).Value - marge <= param4 Or param4 = 0) Then
                        If (.Cells(l, 9).Formula = "" Or .Cells(l, 9).Value + marge >= param4 Or param4 = 0) Then
                        If (.Cells(l, 10).Formula = "" Or .Cells(l, 10).Value - marge <= param5 Or param5 = 0) Then
                        If (.Cells(l, 11).Formula = "" Or .Cells(l, 11).Value + marge >= param5 Or param5 = 0) Then
                        If (.Cells(l, 12).Formula = "" Or .Cells(l, 12).Value <= param6 Or param6 = 0) Then
                        If (.Cells(l, 13).Formula = "" Or .Cells(l, 13).Value >= param6 Or param6 = 0) Then
                        If (.Cells(l, 16).Formula = "" Or .Cells(l, 16).Value - marge <= param7 Or param7 = 0) Then
                        If (.Cells(l, 17).Formula = "" Or .Cells(l, 17).Value + marge >= param7 Or param7 = 0) Then
                        If (.Cells(l, 18).Formula = "" Or .Cells(l, 18).Value - marge <= param8 Or param8 = 0) Then
                        If (.Cells(l, 19).Formula = "" Or .Cells(l, 19).Value + marge >= param8 Or param8 = 0) Then
                        If (.Cells(l, 22).Formula = "" Or .Cells(l, 22).Value - marge <= param9 Or param9 = 0) Then
                        If (.Cells(l, 23).Formula = "" Or .Cells(l, 23).Value + marge >= param9  Or param9 = 0) Then
                            ReDim Preserve propositions(UBound(propositions) + 1)
                            propositions(UBound(propositions)) = l
Community
  • 1
  • 1
Tibo
  • 383
  • 5
  • 27
  • 1
    if you share you code maybe your `If` of other loops can be replaced with `Select Case`, and `Match` , etc.. – Shai Rado Feb 07 '17 at 09:34
  • 2
    I think your answer is here: http://stackoverflow.com/a/7015575/78522 – iDevlop Feb 07 '17 at 09:36
  • Also some interesting workarounds here: http://stackoverflow.com/q/3242560/78522 – iDevlop Feb 07 '17 at 09:38
  • 1
    in VBA `If` statement doesn't short circuit. You may want to use `Select Case` construct or, if all those conditions are about selecting records from a database, _filter_ the database (e.g.: with `AutoFilter()` or the likes) – user3598756 Feb 07 '17 at 09:41
  • As for the `Select Case` construct, I remember @Comintern showed a nice exploitation of it for a case very similar to this one. He may hopefully come round and point you to it – user3598756 Feb 07 '17 at 09:43
  • @iDevlop, Thanks. I was indeed looking for "short-circuit evaluation". Too bad VBA doesn't do that. – Tibo Feb 07 '17 at 10:12
  • @user3598756, Yes, I'm filtering records from a database (actually, a single table which I copied in a sheet). But filtering seems quite difficult if not impossible for my case (e.g. `If ignore_param1 Or Left(.Cells(l, 2).Formula, Len(param1)) = param1 Then` where ignore_param1 is a boolean) – Tibo Feb 07 '17 at 10:26
  • You can always look at the logic and group it, and use multiplication and addition to replace some of the ANDs and ORs `((1=1)+(2=3))` will equal true as the + is an OR and `((1=1)*(2=3))` will be false as * is an AND. You could look at the logic in a way that says if cond1 and cond17 are true, then it doesn't matter what any of the other 22 are and perhaps do logic off this derived Boolean. – Nathan_Sav Feb 07 '17 at 10:51
  • @Tibo, when conditions are hard to be restrained in the format required by the filtering method you could use helper columns. But if your database is greater than a "convenient" amount then you should use real databases instead of excel – user3598756 Feb 07 '17 at 11:02
  • 1
    @Nathan_Sav although the `((1=1)+(2=3))` will be equal to `((1=1) or (2=3))` because True=-1 and False=0 **but** it will not prevent the second part of condition to be checked (no performance gain) – S.Serpooshan Feb 07 '17 at 11:03
  • @Nathan_Sav, I'm not sure I understand you. My problem is that I what to avoid as much as possible using `And` and `Or` logic. I've could have written a single `If Cond1 And (Cond2a or Cond2b) And Cond3... And Cond24` statement, but computing the conditions (e.g. `If ignore_param1 Or Left(.Cells(l, 2).Formula, Len(param1)) = param1 Then`) seems to be processor intensive. That why I instead ordered the conditions so that cond**1** is the least probable, then cond**2** is... so that as few conditions as possible are actually computed. – Tibo Feb 07 '17 at 11:04
  • @user3598756, I'm actually looking into requesting a real database (or a copy of the production one). But according to Windows' help, `Get External Data` \ `From SQL Server` seems to **import** the data in a sheet. If I import the whole table, I still have to filter the records (thus run my sub). If I want the SQL server to filter the record, I'd need to fire 250 SQL requests (which I doubt would be performant)... But I'm not sure I'm competent enough for that topic... – Tibo Feb 07 '17 at 11:16
  • It wasn't for a performance game, it was more suggesting looking at a logic table with 24 inputs, that could show redundancy in the need to check for one input until 15 others have been checked, but initially could be checked 2nd in the current logic state, so perhaps taking a step back and looking at the logic again may help. – Nathan_Sav Feb 07 '17 at 11:42
  • @Nathan_Sav, I don't follow you with the _logic table_ (?) and _redundancy_.I added my statement code in the post. – Tibo Feb 07 '17 at 15:55
  • I think the performance hit is not because of the multiple nested ifs, but because you are calling spreadsheet functions all along. Try to place your working area in a 2D VBA array and run your code again. The results will be much better performance-wise. – Ioannis Feb 08 '17 at 07:47
  • @loannis, I was indeed advised to laod the entire data sheet in an 2D array and run the filtering on the array. I'm currently testing that. – Tibo Feb 08 '17 at 08:28
  • @loannis, I tested with loading data in a 2D-array. This single modification make my Sub run more than 10 times quicker (less than 2s vs 23s). My problem was clearly VBA reading/accessing data from the sheet (rather than VBA computing the IF statement.). – Tibo Feb 08 '17 at 21:09

1 Answers1

3

instead of or, you can use Select Case with comma seperated list of conditions as in following:

'If condition2a Or condition2b Then

Select Case True
Case condition2a, condition2b 'here comma means lazy 'OR' (like as OrElse in vb.net)
  's = s + 10
Case Else
  's = s + 20
End Select

Also, there may be many points to improve your macro performance if we can see your code. instantly, the redim of array to add one more item to it may be time consuming in a loop:

ReDim Preserve propositions(UBound(propositions) + 1)

you may consider to increase its ubound as 10 or 100 items each time you reach its length (to reserve some space for next probable uses), but keep the actual upper bound index in a variable...


Update:

as you add some part of your code, i can suggest you to use some helper function for each if as following:

to replace x<param if's:

If (.Cells(l, 6).Formula="" Or .Cells(l, 6).Value-marge<=param3 Or param3=0) Then ...

with something like as:

If test(.Cells(l, 6).Value, marge, param3) Then ...
'or without '.Value': If test(.Cells(l, 6), marge, param3) Then ...

we can define this function:

Function testLesser(v As Variant, marge As Double, param As Double) As Boolean

    'testLesser = (v = "" Or v - marge <= param3 Or param3 = 0)

    If v = "" Then
    ElseIf v - marge <= param Then
    ElseIf param = 0 Then
    Else
                testLesser = False: Exit Function
    End If
    testLesser = True

    '** Another option (using Select Case):
    'Select Case True
    'Case v = "", v - marge <= param, param = 0
    '    testLesser = True
    'Case Else
    '    testLesser = False
    'End Select

End Function

and similar for other type (greater than) of ifs:

If (.Cells(l, 7).Formula="" Or .Cells(l, 7).Value+marge>=param3 Or param3=0) Then ...

we have:

Function testGreater(v As Variant, marge As Double, param As Double) As Boolean

    'testGreater = (v = "" Or v + marge >= param Or param = 0)

    If v = "" Then 'testLesser = True
    ElseIf v + marge >= param Then 'testLesser = True
    ElseIf param = 0 Then 'testLesser = True
    Else
                testLesser = False: Exit Function
    End If
    testLesser = True

    '** Another option (using Select Case):
    'Select Case True
    'Case v = "", v + marge >= param, param = 0
    '    testLesser = True
    'Case Else
    '    testLesser = False
    'End Select

End Function

So, the code will look like as:

'If (.Cells(l, 6).Formula = "" Or .Cells(l, 6).Value - marge <= param3 Or param3 = 0) Then
'If (.Cells(l, 7).Formula = "" Or .Cells(l, 7).Value + marge >= param3 Or param3 = 0) Then
'If (.Cells(l, 8).Formula = "" Or .Cells(l, 8).Value - marge <= param4 Or param4 = 0) Then
'If (.Cells(l, 9).Formula = "" Or .Cells(l, 9).Value + marge >= param4 Or param4 = 0) Then
'...

If testLesser(.Cells(l, 6), marge, param3) Then
If testGreater(.Cells(l, 7), marge, param3) Then
If testLesser(.Cells(l, 8), marge, param4) Then
If testGreater(.Cells(l, 9), marge, param4) Then
'...

My real test shows its faster! (and obviously, its also more readable code)

Note:

its very important to arrange the if conditions such that you get final condition as soon as you can! for example if cell values are usually empty, put that condition at first in our test function, but if param = 0 is generally true, bring it as first condition check...

this is the rule for x OR y criteria. for 'x AND y' criteria, it is the reverse! the most rare case must be at first to quickly filter the results. in your code, i see you arrange the nested if's from Cells(l, 6) to Cells(l, 23). I don't know if this is best for your situation. it depends on your data and usual cases, so consider revising order of those nested if's if you know some are usually false...

Another Tip:

instead of using With Sheets("Sheet1"), cache it within a variable, this can improve the performance!

Dim mySheet As Worksheet
Set mySheet = Sheets("Sheet1")
With mySheet 'Sheets("Sheet1")

my test shows this simple reference change is faster about 10%. you may think of other similar changes when working with sheets, ranges, cells...

Note: if we can define marge as global or sheet level var, we could remove it from function params but it seems that it doesn't have sensible effect...

Last Update:

as suggested by @Ioannis in comments (see also this ref) when working with a large range of cells, its better to load values into a 2D Array and using it instead of direct access to the Cells:

myArray = Sheets("Sheet1").Range("A1:AG15000").Value

then use that Array for read/writes as:

myArray(row, col) = myArray(row, col) + 1 
'row = 1 to UBound(myArray, 1) 'First array dimension is for rows
'col = 1 to UBound(myArray, 2) 'Second array dimension is for columns

finally when you finished you can update the entire range reversely:

Sheets("Sheet1").Range("A1:AG15000") = myArray
S.Serpooshan
  • 7,608
  • 4
  • 33
  • 61
  • Regarding the **array redim point**, I actually ran a test (called 5000 times) where I created once a 10000-array VS redim the array every line OR redim every 1000 lines `(If Int(l / 1000) * 1000 = l Then ReDim Preserve tableau3(l * 10)`). Redim every time is for sure the worst performing option. But since, I use the size of the array to filter the results, pre-dimensioning is not an option for me. – Tibo Feb 07 '17 at 10:07
  • Regarding **Select Case**, the 24-deep nested `If` statement code is already hardly readable. Nesting some `Case Select` inside the `If`... I'm not sure I'm even willing to do so... – Tibo Feb 07 '17 at 10:17
  • Keep the actual **upper bound index in a variable**. At the beginning, my code was doing so, but since I used this array a lot (in different sub, modules and userform), I got rid of my _array_size_ variable and switched to Ubound... :-( – Tibo Feb 07 '17 at 10:35
  • 1
    about the 24 nested ifs, i can't do anything without seeing them if they could be merged or revised for better performance. you need to convert just those that have some `OR` into the select case (not all of them) and this is the cost you should pay to gain better results. regarding the array_size variable, it is the same: to have better performance you have to write some more complex code (although i don't think using such variable be so hard if implemented correctly). but the final improvement effect depends on many things in your scenario to be measured by you. – S.Serpooshan Feb 07 '17 at 10:58
  • OK, so I just modify the code to totally avoid the ***array redim*** (by declaring an fixed-sized (500) array and exiting if more than 500), but there is almost no gain in performance, since most of the time (>90%), the redim is called only once (which is what I'm trying to achieve with the nested if statement) ! But thanks for the input. (Actually, I think I might even exit conditionnaly if Ubound >1...) – Tibo Feb 07 '17 at 12:49
  • Thanks for you in-deep analysis and your tips. --- I should have use functions form the start, for readability but also because it requests the formula/value once (instead of twice). --- I was indeed advised to load the entire data sheet in an 2D array and run the filtering on the array. I'm currently testing that. I'll add the helper function then. – Tibo Feb 08 '17 at 09:28
  • I tested with loading data in a 2D-array. This single modification make my Sub run more than 10 times quicker (less than 2s vs 23s). My problem was clearly VBA **reading/accessing data from the sheet** (rather than VBA computing the IF statement.). Is `Set mySheet = Sheets(X)` equivalent of using `myArray = Sheet(X).Range("A1:AG15000").Value`in this regard ? I tought using `Set mySheet = Sheets(X)` was kind of a reference (hence working as `With Sheets(X)`) and not loading the sheet in memory. – Tibo Feb 08 '17 at 10:09
  • glad to see your luck. no, it doesn't load as 2D array. and it is absolutely required when enumerating large range of cells (i'll add this tip to the ans)... but my answer was focused on your question text as how to fine-tune those nested IF blocks and i provided the solution mostly for it aspect ;D – S.Serpooshan Feb 08 '17 at 10:17