Forum Discussion
Solved
Error: run time error 5 invalid procedure call or argument when VBA is executed
I would like to extract the latest excel file based on the current month from the filepath. if based on the current month, if there is no file, the file extracted will be based on the previous month...
hrh_dash There is not much code to be shortened, but...
- Note that your first If test is not necessary. Line 13 has just constructed a string value that ends with a three-character month abbreviation. So the test for "not equal to a backslash" will always be true. As a result, you can remove lines 26 and 15. And if you change 13 to append the backslash there, you can also remove (what is shown as) line 17.
- Look again at lines 19 and 21. You put the result of the Dir function into strFileExist, but you check...? But it seems (from code further down) that you should not Exit Sub if no file was found, right? (I.e., if you don't find a file for the current month, you want to look at files for the previous month.) So three more (nonblank) lines of code could be removed.
- You assign values to findFile, but then don't do anything else with it. If you are just using that variable while you develop and test the code, and plan to remove it when everything works, that's fine. Technically, you do not need the "findFile = False" statement because Boolean variables are automatically initialized to False, just like numeric variables are automatically initialized to zero.
- This will not reduce the number of lines of code, but... As a matter of good practice, you should try to minimize the future maintenance work. So rather than putting two literals in for the base directory (something on your V: drive, which may change in the future), make that a variable (if you plan to soon change your code to obtain the value, e.g., by reading a file of parameters) or a constant (if your code should not obtain it). Then use the variable or constant when building your argument for the Dir function:
Const strBASE_DIRECTORY = "V:\Finance\Systems-Risk-ERM\OrderToCash\C2C\Reporting\Ageing, Billing & Collection\BSCS Ageing\" ... strFile = strBASE_DIRECTORY & prevdate_year & "\" & prevdate & "\" ... strFile = strBASE_DIRECTORY & prevdate_year & "\" & prevdate_prev & "\"But I would encourage you to add lines, specifically lines of comments. Briefly describe what your blocks of code are doing, like:
'---- Locate the Excel workbook in the current-month folder with the ' latest modification date; capture its filename and modification date. ... '---- If no such file was found, locate the Excel workbook in the previous- ' month's folder with the latest modification date; capture its ' filename and modification date. ...And as you write comments (i.e., what you expect the code to do), you are more likely to notice deficiencies. (E.g., what happens when "previous month" occurred in the previous year?) And you become more likely to use appropriate and meaningful variable names ("I'm looking at you, "prevdate").
BTW, in your last Debug.Print statement, as an alternative to concatenating the two variables, note what you get when you separate them with a comma.
hrh_dash When you get that error (or any other), examine the value of relevant variables, e.g., by typing "Debug.Print strFileExist" or "? strFileExist" (without the quotes, of course) in the Immediate window, or typically by just hovering your mousepointer over the variable name. Or, you can highlight a whole expression and click the menu item "Debug | Add Watch..." I think you will be surprised at what the Dir function is returning.
Also, to aid people (yourself and others) in reading source code, note that you can split your VBA statements onto multiple lines (basically, at word boundaries, unless it's within a literal value) by leaving a space and underscore at the end of each line that is continued to the next. E.g.:
filenameWOExt = Mid(strFileExist, InStrRev(strFileExist, "\") + 1 _
, InStrRev(strFileExist, ".") - InStrRev(strFileExist, "\") - 1) 'error _
keeps popping up from this line
SnowMan55 , went to debug.print very variable and add in a Boolean and copied the do while loop.
the code works like how i wanted but is there a way to shorten the code?
Dim strFile As String Dim strFileExist As String Dim fileDate As Date Dim lastDate As Date Dim lastFile As String Dim MonthNum As Integer Dim findFile As Boolean prevdate = Format(DateAdd("M", 0, Now), "Mmm") prevdate_year = Format(DateAdd("Y", 0, Now), "YYYY") prevdate_prev = Format(DateAdd("M", -1, Now), "Mmm") strFile = "V:\Finance\Systems-Risk-ERM\OrderToCash\C2C\Reporting\Ageing, Billing & Collection\BSCS Ageing\" & prevdate_year & "\" & prevdate If Right(strFile, 1) <> "\" Then strFile = strFile & "\" strFileExist = Dir(strFile & "*.xlsx") If Len(strFile) = 0 Then Exit Sub End If End If Do While Len(strFileExist) > 0 fileDate = FileDateTime(strFile & strFileExist) If fileDate > lastDate Then lastFile = strFileExist lastDate = fileDate End If strFileExist = Dir Loop findFile = FALSE If lastFile = "" Then findFile = TRUE strFile = "V:\Finance\Systems-Risk-ERM\OrderToCash\C2C\Reporting\Ageing, Billing & Collection\BSCS Ageing\" & prevdate_year & "\" & prevdate_prev & "\" strFileExist = Dir(strFile & "*.xlsx") Do While Len(strFileExist) > 0 fileDate = FileDateTime(strFile & strFileExist) If fileDate > lastDate Then lastFile = strFileExist lastDate = fileDate End If strFileExist = Dir Loop End If 'Debug.Print strFile 'Debug.Print strFileExist 'Debug.Print fileDate Debug.Print strFile & lastFile End Subhrh_dash There is not much code to be shortened, but...
- Note that your first If test is not necessary. Line 13 has just constructed a string value that ends with a three-character month abbreviation. So the test for "not equal to a backslash" will always be true. As a result, you can remove lines 26 and 15. And if you change 13 to append the backslash there, you can also remove (what is shown as) line 17.
- Look again at lines 19 and 21. You put the result of the Dir function into strFileExist, but you check...? But it seems (from code further down) that you should not Exit Sub if no file was found, right? (I.e., if you don't find a file for the current month, you want to look at files for the previous month.) So three more (nonblank) lines of code could be removed.
- You assign values to findFile, but then don't do anything else with it. If you are just using that variable while you develop and test the code, and plan to remove it when everything works, that's fine. Technically, you do not need the "findFile = False" statement because Boolean variables are automatically initialized to False, just like numeric variables are automatically initialized to zero.
- This will not reduce the number of lines of code, but... As a matter of good practice, you should try to minimize the future maintenance work. So rather than putting two literals in for the base directory (something on your V: drive, which may change in the future), make that a variable (if you plan to soon change your code to obtain the value, e.g., by reading a file of parameters) or a constant (if your code should not obtain it). Then use the variable or constant when building your argument for the Dir function:
Const strBASE_DIRECTORY = "V:\Finance\Systems-Risk-ERM\OrderToCash\C2C\Reporting\Ageing, Billing & Collection\BSCS Ageing\" ... strFile = strBASE_DIRECTORY & prevdate_year & "\" & prevdate & "\" ... strFile = strBASE_DIRECTORY & prevdate_year & "\" & prevdate_prev & "\"But I would encourage you to add lines, specifically lines of comments. Briefly describe what your blocks of code are doing, like:
'---- Locate the Excel workbook in the current-month folder with the ' latest modification date; capture its filename and modification date. ... '---- If no such file was found, locate the Excel workbook in the previous- ' month's folder with the latest modification date; capture its ' filename and modification date. ...And as you write comments (i.e., what you expect the code to do), you are more likely to notice deficiencies. (E.g., what happens when "previous month" occurred in the previous year?) And you become more likely to use appropriate and meaningful variable names ("I'm looking at you, "prevdate").
BTW, in your last Debug.Print statement, as an alternative to concatenating the two variables, note what you get when you separate them with a comma.
- , thank you sir for the insightful review!
