[X]My Assistant

[aggiemarine07]

So I currently use the below code to import about 500 excel spreadsheets into my Access DB but the process takes my computer about 35 minutes to do all of the imports. I was wanting to see if anyone had recommendations that would potentially speed up the process and reduce the amount of time it takes. Here is the code:

CODE

Function Import_Excel(myPath As String, myFile As String, myExt As String, myTable As String, Optional myRange As String, Optional mySQLTable As String, Optional mySQLField As String)

DoCmd.SetWarnings False
DoCmd.RunSQL "DELETE * FROM WrongSheetErrors"
Do While myFile <> ""
  If myFile Like myExt Then
  
   On Error Resume Next
    DoCmd.TransferSpreadsheet acImport, , myTable, myPath & myFile, True, myRange
        If Err.Number = 2391 Then ' <>0 // DoCmd.RunSQL "INSERT INTO [WrongSheetErrors] ([FileName],[FileErrorNumber],[FileErrorDescription],[FileErrorSource]) VALUES ('" & myFile & "', " & Err.Number & ", " & Err.Description & ", " & Err.Source & ");"
                Dim strSQL As String
                strSQL = "INSERT INTO WrongSheetErrors ([FileName], [FileErrorNumber],[FileErrorDescription]) VALUES (" & _
                         Chr(34) & myFile & Chr(34) & ", " & _
                         Err.Number & ", " & _
                         Chr(34) & Err.Description & Chr(34) & ");"
                'DoCmd.SetWarnings False
                DoCmd.RunSQL strSQL
                'DoCmd.SetWarnings True
                Resume Next
            Else: Resume Next
            End If
      
Else
  Name myPath & myFile As myPath & myFile
End If
  myFile = Dir()
Loop

If DCount("*", "WrongSheetErrorsQuery") > 0 Then
    DoCmd.OpenQuery "WrongSheetErrorsQuery"
        Else: MsgBox "All Good"
    End If
    
DoCmd.SetWarnings True

End Function

[theDBguy]

Hi. Not sure if there's anything else you can do to speed it up. Processing 500 files will just take some time by default. For example, if you take the biggest file and import it manually, how long does it take? Just curious...

[aggiemarine07]

Yeah I understand it will take time, just looking to see if there is a more efficient way for me to implement it

I average 22 seconds a file.

[dmhzx]

would currentdb.execute be faster than docmd.runsql?

Would it help if you used actual error trapping, so you only looked at the error code if there was an error? Checking for an error 500 time may be slower than waiting for Access to tell you there is one? And within the trap, you can trap and report any errors.

You current code only traps one possible error if there are any others it will still try to import. (Maybe one of the files is password protected? Excel thinks it's already open?, IS it corrupted?

I normally do that in conjunction with labels in the code

On a quick glance I would put a label in just above the Loop Line, or perhaps the "myFile = Dir()" and call it LoopEnd:

Move all your current error working into the Trap area, and Resume LoopEnd:


Let me know if you'd like more detail

[theDBguy]

I average 22 seconds a file.

That's over 2 hours, on average, for 500 files. So, you're already doing better with it only taking 35 minutes.

[cheekybuddha]

Hi,

Your code is a little strange ... !

CODE

' ...
  If myFile Like myExt Then
    ' Do import or write error
  Else
    Name myPath & myFile As myPath & myFile
  End If
' ...

What is going on in that If statement? Since you are not using a wildcard in the comparison pattern then it will only evaluate to True if myFile = myExt (although case insensitively). Is that intended?

If the result is False then the expression Name myPath & myFile As myPath & myFile doesn't appear to do anything useful, except rename the file to what it was before!

What am I missing?

d

[dmhzx]

CheekyBuddha:

Yes I was just about to come back on that one.

I also wondered about the line

CODE

myFile = Dir()

Personally I would have written this as a sub rather than a function: It is not returning anything. - I know it makes no practical difference I just like to know that my functions are there to return something. It's one my few policies.


David

[cheekybuddha]

David, It looks like this might have been called originally in the code that calls this function, and this line is used to iterate over any more files in the folder.

I agree, it's messy. But I wanted to try and work out what else is happening before tidying up!

[kfield7]

A tip, when passing parameters to a function or subroutine, it helps if you comment the purpose of the parameters.
By its name, myExt is implied to be a file extension, such as ".xls".
Is your routine looking for only ".xls", or whatever is passed to it? ".xlsx" ".xlsm" ".xls" etc.?

instead of like, maybe be more explicit:
if right(myFile,4) = right(myExt,4) then 'this captures both ".xls" and ".xlsx" types of matches.

Another approach
if instr(1, myFile, myExt, 0) >0 'however, this could give erroneous results if the filename is "the.xlsx.file.description.doc", and it could also error using like. Unless of course you simply won't have any files with that type of name.

[aggiemarine07]

Thank you everyone for the response and help, I'll respond to everyone questions below:

@dmzhx:

Would it help if you used actual error trapping,

I thought thats what I did there as it exports anything with an error to a table I have within the DB. I only specificed Err2391 because that was the only error I was getting upon import. The Sheets are protected but there is not password (its designed to prevent people from making accidental adjustments to the file)

On a quick glance I would put a label in just above the Loop Line, or perhaps the "myFile = Dir()" and call it LoopEnd:

what do you mean by this?

I also wondered about the line

This line could probably be remove as well, as its likely a holdover from my cobbled together code. I am not near as much of an expert as yall with writing code (I'm an novice at best) thus why I am turning to yall since yall are way better at this than me

@cheekybuddha:

What is going on in that If statement?

I could probably remove that statement I cobbled this together from multiple places on the internet so it might be some hangover from the original stuff.

I agree, it's messy.

I concur 100% :-D

@kfield7:
Thanks for the tips! I am always open to learning and getting better at writing VBA as I am well aware of my shortcomings

Is your routine looking for only ".xls", or whatever is passed to it? ".xlsx" ".xlsm" ".xls" etc.?

My routine is only looking for XLSB files in it as those will be the only files in the folder; nothing else.

Any recommends yall have to make this more efficient would be more than appreciated and thank you again for taking the time to read my question and help me out!

[cheekybuddha]

Actually, re-looking at it my guess is that parameter myExt will probably contain the wildcards when passed. eg "*.xls*"

It will work, but isn't very clear what's going on!!! And if that is the case then the line I questioned is necessary - don't remove it!

My advice is: if it works OK, then leave it alone!

It could be more 'efficient' if it were re-written to 'self-document' what is going on, or with fewer lines of code, but otherwise I don't think you'll be able to improve the execution time of the code.

hth,

d

[dmhzx]

Here is an outline covering error trapping. It's basic, but if you like it I'll have a go at putting a demon together.
This is just how I normally do it, but it puts in some structure, and if ti do get any more error you are told about them and can code as appropriate.

CODE

On error goto Trap:   (Put this just after your Dims)

    code

    code

Do until something
   code
Loopend:
    move to the next whateever
loop

CleanExit:



on error resume next  (This says to ignore any errors during the clean up. So if you try to close something that isn't open, for example, you won't get an error reported)

do your cleanup code


exit sub   (This is needed to stop the normally running code from reaching the error trapping section)

Trap:  (If there is an error the code jumps to here)

if err.number =  2391 then  (This is the error you know about, so your existing code for that goes in here)
     capture the error

    Resume Loopend:  (If you get that error you want to go to the next sheet, and continue round the loop)

else

    msgbox err.number & " - " & err.description (This will tell you want the error number was and its description)

    resume CleanExit:  (CleanExit is where the final code goes if there is any. It may be just setting various objects to nothing. )

end if

end sub

I hope that gives you some pointers. The reason I tend to code like this as a standard structure is simply that I find it helps me when I revisit code a year or so later. Just like all the Dims are together, so is all the error handling, and your are in control of when you want to trap errors and when you want to ignore them.


Would you like me to have a go at restructuring the code for you? I won't be able to test it, or would you like to use it as a learning experience?

I hope that is some help. The code may not run any faster but you will find it easier to follow and to support.
I think case I would also put in a comment after the line with LIKE in. As Cheely Buddha pointed out Like without any wildcards means equals. So we think what you're doing here is passing "*.xls*" or similar, and so the line makes a lot of sense.
*.xls* would allow you to process almost any excel file.

[aggiemarine07]

thanks everyone for all of your help, especially with how to properly write an error handler.

After doing a lot more digging on it, I noticed that my excel files have ballooned from 300kb to 2.5mb and that this more than likely the culprit behind the slow imports. Im going to go through them all to figure how this happened and try to reduce the file sizes.

[dmhzx]

Excel files can balloon for all sorts of reasons.

But 300K to 2.5 meg ??
Do let us know how you get on, and let me know if a basic error trap demo file would help.

One thing I did forget to add to the code is a line AFTER the 'Resume' wherever that simply says

Resume
The code will normally never reach that line, but if you do get an error you haven't seen before, you can set the next line to resume, hit F8, and see the line that caused the problem.

I have found this VERY useful during the development of an app.