HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Extracting key information from a directory of XML files

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
directoryxmlfilesextractingfromkeyinformation

Problem

I have written some VBA to loop through a directory of 15,000 XML files and extract key information from these files into Excel - this is unbearably slow. Can someone please assist me in optimising the module?

Sub ImportXMLtoList()
 Application.DisplayAlerts = False
 Application.ScreenUpdating = False
 oldStatusBar = Application.DisplayStatusBar
 Application.DisplayStatusBar = True

 Dim myDir As String
 Dim StrFile As String
    StrFile = Dir("\\Ntsydfsp152\shared\Temp\Lucsan\LMIG\Output\XML\SBL\Staging\")
    myDir = "\\Ntsydfsp152\shared\Temp\Lucsan\LMIG\Output\XML\SBL\Staging\"
    myFile = "LMIG_List_XML_Files.xlsm"

 'clear sheet1
 Sheets(1).Range("a2:g30000").Clear
 x = 2

 Do While Len(StrFile) > 0

 Application.StatusBar = StrFile & " Row: " & x
    Sheets(1).Cells(x, 1).Value = StrFile
    Workbooks.OpenXML Filename:=myDir & StrFile, LoadOption:=xlXmlLoadImportToList
    temp2 = ActiveWorkbook.Sheets(1).Range("g2")
    temp3 = ActiveWorkbook.Sheets(1).Range("e2")
    temp4 = ActiveWorkbook.Sheets(1).Range("ak3")
    temp5 = ActiveWorkbook.Sheets(1).Range("av4") & "/" & ActiveWorkbook.Sheets(1).Range("ax4") & "/" & ActiveWorkbook.Sheets(1).Range("aw4") & "/" &     ActiveWorkbook.Sheets(1).Range("ay4")

    ActiveWorkbook.Close

    Sheets(1).Cells(x, 2) = temp2
    Sheets(1).Cells(x, 3) = temp3
    Sheets(1).Cells(x, 4) = temp4
    Sheets(1).Cells(x, 5) = temp5
    x = x + 1

    StrFile = Dir
 Loop

 Application.DisplayAlerts = True
 Application.ScreenUpdating = True
 Application.StatusBar = False
 Application.DisplayStatusBar = oldStatusBar

End Sub

Solution

A few short comments:
Use Option Explicit:

Why? because It makes thinks easier by orders of magnitude when developing.

You get more useful error messages and it becomes harder to misspell variable names without realizing.

That brings us right to the next:
Variable Names:

Your variable names are not consistently cased. You are mixing up "camel casing" (e.g. myDir) and "Pascal Casing" (e.g. StrFile)

Also temp 1 through 4 are not really good variable names. Neither is x, but hey ;)
Error Handling:

You should really make sure your "Application" is in a clean state after exit, even when you got unexpected errors:

Sub ImportXMLToList()
    On Error GoTo ErrorHandler

    ' Code goes here: 

:CleanExit
    Application.DisplayAlerts = True
    Application.ScreenUpdating = True
    Application.StatusBar = False
    Application.DisplayStatusBar = oldStatusBar

    Exit Sub
:ErrorHandler
    'Close resources, maybe errormessage
    Resume CleanExit
End Sub

Code Snippets

Sub ImportXMLToList()
    On Error GoTo ErrorHandler

    ' Code goes here: 

:CleanExit
    Application.DisplayAlerts = True
    Application.ScreenUpdating = True
    Application.StatusBar = False
    Application.DisplayStatusBar = oldStatusBar

    Exit Sub
:ErrorHandler
    'Close resources, maybe errormessage
    Resume CleanExit
End Sub

Context

StackExchange Code Review Q#64484, answer score: 6

Revisions (0)

No revisions yet.