patternMinor
Excel VBA - Get Folder Names
Viewed 0 times
excelnamesgetfoldervba
Problem
I need to get folder names with the path for which I need to search the directory in a server with about 6000 folders. I have the following snippet of code to run through the folder and get the folder names with path. It works fine in a local directory but when I run the same code on a server directory it fails after about printing 86 folder names. The code works absolutely fine but stops working when run on a server directory with about 6000 folders. Excel freezes up and seems like it hangs.
Private Sub PrintFolders()
Dim objFSO As Object
Dim objFolder As Object
Dim objSubFolder As Object
Dim i As Integer
Application.StatusBar = ""
'Create an instance of the FileSystemObject
Set objFSO = CreateObject("Scripting.FileSystemObject")
'Get the folder object
Set objFolder = objFSO.GetFolder("C:\Temp")
i = 1
'loops through each folder in the directory and prints their names and path
On Error GoTo handleCancel
Application.EnableCancelKey = xlErrorHandler
MsgBox "This may take a long time: press ESC to cancel"
For Each objSubFolder In objFolder.subfolders
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
'print folder name
Cells(i + 1, 1) = objSubFolder.Name
'print folder path
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
Next objSubFolder
handleCancel:
If Err = 18 Then
MsgBox "You cancelled"
End If
End SubSolution
UX
Before diving into the code, I have a few points to mention about the user experience of it.
Readability
The code itself reads a bit like a clogged script. Indentation is insufficient and inconsistent, and vertical whitespace is completely inexistent. This code needs to breathe a little - here's your code, without any other changes:
Comments
There are way too many comments in that code. Good, valuable comments explain why code is doing what it does - the code itself should be self-explanatory about the what.
I would simply remove them... all.
Isn't too far from:
Non-Responsiveness
You're running a pretty tight loop here:
You never give Excel a chance to breathe and actually respond to the events you're sending - namely updating the statusbar and listening to ESC keypresses.
This would fix it:
...but it comes with a pretty high price: updating the status bar and listening for keypresses at each and every iteration will considerably slow down execution. How about reorganizing it a little, and only do that once every 10 iterations? Avoid magic numbers, give that value a meaningful name and assign it to a constant:
Now, you're writing to worksheet cells in a loop, while keeping Excel relatively responsive. Are you sure you're writing to the correct worksheet?
Without an object reference,
Performance
I'm willing to bet anything that your biggest bottleneck isn't the FSO, but actually writing to the cells. Of course you need to do that.. but do you need Excel to repaint its grid and verify that nothing needs to be recalculated every time you write to a cell? Of course you don't.
Switch it off:
Now, whenever you do that, you must handle runtime errors appropriately.
Error Handling
Your error handling is copy-pasted from MSDN, which is only an example to illustrate how a specific feature works - these examples are always focused on a very specific feature, and never about best practices.
Your method should fail cleanly, and correctly cleanup whether or not an error is raised - every error-sensitive method should be templated something like this:
```
Sub DoSomething()
On Error GoTo CleanFail
'implementation code here
CleanExit:
'cleanup code here
Exit Sub
CleanFail:
'error-handling co
Before diving into the code, I have a few points to mention about the user experience of it.
- You're not resetting the
Application.Statusbarbefore the procedure exits, which means whenever the code runs to completion, the Excel status bar remains "frozen" with the last processed folder.
- You're warning the user that the operation may take a long time, regardless of how many folders/subfolders there actually are.
- Message boxes are bare-bones, without a title or an icon, and punctuation is missing from the message strings.
Readability
The code itself reads a bit like a clogged script. Indentation is insufficient and inconsistent, and vertical whitespace is completely inexistent. This code needs to breathe a little - here's your code, without any other changes:
Private Sub PrintFolders()
Dim objFSO As Object
Dim objFolder As Object
Dim objSubFolder As Object
Dim i As Integer
Application.StatusBar = ""
'Create an instance of the FileSystemObject
Set objFSO = CreateObject("Scripting.FileSystemObject")
'Get the folder object
Set objFolder = objFSO.GetFolder("C:\Temp")
i = 1
'loops through each folder in the directory and prints their names and path
On Error GoTo handleCancel
Application.EnableCancelKey = xlErrorHandler
MsgBox "This may take a long time: press ESC to cancel"
For Each objSubFolder In objFolder.subfolders
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
'print folder name
Cells(i + 1, 1) = objSubFolder.Name
'print folder path
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
Next objSubFolder
handleCancel:
If Err = 18 Then
MsgBox "You cancelled"
End If
End SubComments
There are way too many comments in that code. Good, valuable comments explain why code is doing what it does - the code itself should be self-explanatory about the what.
I would simply remove them... all.
'Create an instance of the FileSystemObject
Set objFSO = CreateObject("Scripting.FileSystemObject")Isn't too far from:
'increment i:
i = i + 1Non-Responsiveness
You're running a pretty tight loop here:
For Each objSubFolder In objFolder.subfolders
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
Cells(i + 1, 1) = objSubFolder.Name
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
Next objSubFolderYou never give Excel a chance to breathe and actually respond to the events you're sending - namely updating the statusbar and listening to ESC keypresses.
This would fix it:
For Each objSubFolder In objFolder.subfolders
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
Cells(i + 1, 1) = objSubFolder.Name
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
DoEvents
Next objSubFolder...but it comes with a pretty high price: updating the status bar and listening for keypresses at each and every iteration will considerably slow down execution. How about reorganizing it a little, and only do that once every 10 iterations? Avoid magic numbers, give that value a meaningful name and assign it to a constant:
Const IterationsToUpdate As Integer = 10
For Each objSubFolder In objFolder.subfolders
Cells(i + 1, 1) = objSubFolder.Name
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
If i Mod IterationsToUpdate = 0 Then
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
DoEvents
End If
Next objSubFolderNow, you're writing to worksheet cells in a loop, while keeping Excel relatively responsive. Are you sure you're writing to the correct worksheet?
Cells(i + 1, 1) = objSubFolder.Name
Cells(i + 1, 2) = objSubFolder.PathWithout an object reference,
Cells is referring to the active sheet, which the user is free to change any time as the code runs. This is another UX issue if not a bug.Performance
I'm willing to bet anything that your biggest bottleneck isn't the FSO, but actually writing to the cells. Of course you need to do that.. but do you need Excel to repaint its grid and verify that nothing needs to be recalculated every time you write to a cell? Of course you don't.
Switch it off:
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManualNow, whenever you do that, you must handle runtime errors appropriately.
Error Handling
Your error handling is copy-pasted from MSDN, which is only an example to illustrate how a specific feature works - these examples are always focused on a very specific feature, and never about best practices.
Your method should fail cleanly, and correctly cleanup whether or not an error is raised - every error-sensitive method should be templated something like this:
```
Sub DoSomething()
On Error GoTo CleanFail
'implementation code here
CleanExit:
'cleanup code here
Exit Sub
CleanFail:
'error-handling co
Code Snippets
Private Sub PrintFolders()
Dim objFSO As Object
Dim objFolder As Object
Dim objSubFolder As Object
Dim i As Integer
Application.StatusBar = ""
'Create an instance of the FileSystemObject
Set objFSO = CreateObject("Scripting.FileSystemObject")
'Get the folder object
Set objFolder = objFSO.GetFolder("C:\Temp")
i = 1
'loops through each folder in the directory and prints their names and path
On Error GoTo handleCancel
Application.EnableCancelKey = xlErrorHandler
MsgBox "This may take a long time: press ESC to cancel"
For Each objSubFolder In objFolder.subfolders
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
'print folder name
Cells(i + 1, 1) = objSubFolder.Name
'print folder path
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
Next objSubFolder
handleCancel:
If Err = 18 Then
MsgBox "You cancelled"
End If
End Sub'Create an instance of the FileSystemObject
Set objFSO = CreateObject("Scripting.FileSystemObject")'increment i:
i = i + 1For Each objSubFolder In objFolder.subfolders
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
Cells(i + 1, 1) = objSubFolder.Name
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
Next objSubFolderFor Each objSubFolder In objFolder.subfolders
Application.StatusBar = objSubFolder.Path & " " & objSubFolder.Name
Cells(i + 1, 1) = objSubFolder.Name
Cells(i + 1, 2) = objSubFolder.Path
i = i + 1
DoEvents
Next objSubFolderContext
StackExchange Code Review Q#86441, answer score: 9
Revisions (0)
No revisions yet.