patternMinor
Multiple nested If checks in VBA
Viewed 0 times
checksnestedmultiplevba
Problem
The below code is real and in use, but I've modified it to simplify the process/make it easier to explain.
The purpose of this code is to combine data from multiple data sources. All sources are .xls files, but the names will vary based on the date of creation, and the formatting can vary based on the user.
Because the names are variable, I have opted to prompt the
user to select each of the necessary files, rather than try to code in an auto-selection.
Also, since the end-users like to have things as simple as possible, I've condensed this
process into one function (there are multiple extra functions, all wrapped in this main
function) as opposed to having to manually begin multiple processing steps. Since there
are a number of files involved, I have designed this to have the user pre-open each
of the necessary files (which all live in different locations on a network) before
beginning this process. As a result, I have multiple
has not cancelled mid-way (say, they forgot to open one of the files earlier and didn't
realize until they were prompted for it).
Is there a better way to collect all the files that are needed for processing while allowing the user to cancel at any step in the process?
There is nothing wrong for me to have such deep nesting, but I feel like this may be bad
form. I believe I can put each check all on one line with
also become unwieldy/harder to debug if there was an issue.
I have added comments to the code to explain what may be missing from the example, but feel free to ask if I've missed anything.
Update: I've made it a little nicer by putting all the workbook/sheet assignments after the last if.
```
Option Explicit
Sub Example()
Dim Book0 As Workbook
Dim Sheet0 As Worksheet
Dim Book1 As Workbook
Dim Book2 As Workbook
Dim Sheet2 As Worksheet
Dim Book3 As Workbook
Dim Sheet3 As Worksheet
Dim Book4 As Workbook
Dim Sheet4 As Worksheet
Dim Book5 As Workbook
Dim Sheet5 As W
The purpose of this code is to combine data from multiple data sources. All sources are .xls files, but the names will vary based on the date of creation, and the formatting can vary based on the user.
Because the names are variable, I have opted to prompt the
user to select each of the necessary files, rather than try to code in an auto-selection.
Also, since the end-users like to have things as simple as possible, I've condensed this
process into one function (there are multiple extra functions, all wrapped in this main
function) as opposed to having to manually begin multiple processing steps. Since there
are a number of files involved, I have designed this to have the user pre-open each
of the necessary files (which all live in different locations on a network) before
beginning this process. As a result, I have multiple
If checks to verify that a userhas not cancelled mid-way (say, they forgot to open one of the files earlier and didn't
realize until they were prompted for it).
Is there a better way to collect all the files that are needed for processing while allowing the user to cancel at any step in the process?
There is nothing wrong for me to have such deep nesting, but I feel like this may be bad
form. I believe I can put each check all on one line with
And, but that I think wouldalso become unwieldy/harder to debug if there was an issue.
I have added comments to the code to explain what may be missing from the example, but feel free to ask if I've missed anything.
Update: I've made it a little nicer by putting all the workbook/sheet assignments after the last if.
```
Option Explicit
Sub Example()
Dim Book0 As Workbook
Dim Sheet0 As Worksheet
Dim Book1 As Workbook
Dim Book2 As Workbook
Dim Sheet2 As Worksheet
Dim Book3 As Workbook
Dim Sheet3 As Worksheet
Dim Book4 As Workbook
Dim Sheet4 As Worksheet
Dim Book5 As Workbook
Dim Sheet5 As W
Solution
You can replace all of the Getbook nested ifs with the following. Place this first:
Then below that list what you want to happen:
This replaces the huge amount of ifs with a loop that has the same result. I.E. Operation continues only if every if statement returns true, and if any return false the sub exits.
Edit:
It should also be possible to standardize your Set Book0 Set Sheet0 sections as well by using arrays.
To do that you would do something like this...:
This way once the loop is done all the workbooks have been assigned to the array.
Dim i As Long
For i = 0 To 6
If Not Getbook("Please select the Book" & i & ".", "Please select a sheet within Book" & i & ".", i) Then
Exit Sub
End If
Next iThen below that list what you want to happen:
Set Book0 = Workbooks(sListing(0, 0))
Set Sheet0 = Book0.Sheets(sListing(0, 1))
Set Book1 = Workbooks(sListing(1, 0))
Set Book2 = Workbooks(sListing(2, 0))
Set Sheet2 = Book2.Sheets(sListing(2, 1))
Set Book3 = Workbooks(sListing(3, 0))
Set Sheet3 = Book3.Sheets(sListing(3, 1))
Set Book4 = Workbooks(sListing(4, 0))
Set Sheet4 = Book4.Sheets(sListing(4, 1))
Set Book5 = Workbooks(sListing(5, 0))
Set Sheet5 = Book5.Sheets(sListing(5, 1))
Set Book6 = Workbooks(sListing(6, 0))
Set Sheet6 = Book6.Sheets(sListing(6, 1))
'Do stuff with books/sheets.
MsgBox "Process complete!"This replaces the huge amount of ifs with a loop that has the same result. I.E. Operation continues only if every if statement returns true, and if any return false the sub exits.
Edit:
It should also be possible to standardize your Set Book0 Set Sheet0 sections as well by using arrays.
To do that you would do something like this...:
Dim workbooks(6) As Workbook
Dim sheets(6) As Worksheet
Dim i As Long
For i = 0 To 6
If Not Getbook("Please select the Book" & i & ".", "Please select a sheet within Book" & i & ".", i) Then
Exit Sub
Else
Set workbooks(i) = workbooks(slisting(i, 0))
Set sheets(i) = workbooks(i).sheets(slisting(i, 1))
End If
Next i
'Do stuff with books/sheets.
MsgBox "Process complete!"This way once the loop is done all the workbooks have been assigned to the array.
Code Snippets
Dim i As Long
For i = 0 To 6
If Not Getbook("Please select the Book" & i & ".", "Please select a sheet within Book" & i & ".", i) Then
Exit Sub
End If
Next iSet Book0 = Workbooks(sListing(0, 0))
Set Sheet0 = Book0.Sheets(sListing(0, 1))
Set Book1 = Workbooks(sListing(1, 0))
Set Book2 = Workbooks(sListing(2, 0))
Set Sheet2 = Book2.Sheets(sListing(2, 1))
Set Book3 = Workbooks(sListing(3, 0))
Set Sheet3 = Book3.Sheets(sListing(3, 1))
Set Book4 = Workbooks(sListing(4, 0))
Set Sheet4 = Book4.Sheets(sListing(4, 1))
Set Book5 = Workbooks(sListing(5, 0))
Set Sheet5 = Book5.Sheets(sListing(5, 1))
Set Book6 = Workbooks(sListing(6, 0))
Set Sheet6 = Book6.Sheets(sListing(6, 1))
'Do stuff with books/sheets.
MsgBox "Process complete!"Dim workbooks(6) As Workbook
Dim sheets(6) As Worksheet
Dim i As Long
For i = 0 To 6
If Not Getbook("Please select the Book" & i & ".", "Please select a sheet within Book" & i & ".", i) Then
Exit Sub
Else
Set workbooks(i) = workbooks(slisting(i, 0))
Set sheets(i) = workbooks(i).sheets(slisting(i, 1))
End If
Next i
'Do stuff with books/sheets.
MsgBox "Process complete!"Context
StackExchange Code Review Q#13632, answer score: 6
Revisions (0)
No revisions yet.