Workbook object woes

[remmer75]

I'm trying to set a workbook variable from a file name, but getting run time error 9.

I'm trying to understand VBA a bit better, so even if I have a bit of code that works, it feels like it's a hack, and I'd rather know the proper way of coding it - like why I can't get a workbook object returned from the workbooks collection. It's likely painfully obvious what's wrong with the code, and I'll be laughed at, but wiser

Sub OpenFile()
 
    Dim varFilename As String
    Dim inputWorkbook As Workbook
    varFilename = Application.GetOpenFilename("Excel Files, *.xl*") 'Get open file dialog, filter on files with *.xl*
    If varFilename = "False" Then End 'If no file is selected, end the macro altogether
    Workbooks.Open Filename:=varFilename 'Open file
    Set inputWorkbook = ActiveWorkbook '<-- Works
    Set inputWorkbook = Workbooks(varFilename) '<-- Doesn't work
    Set inputWorkbook = Workbooks.Item(varFilename) '<-- Doesn't work either

End Sub

 

[wigi]

varFilename contains the filename, but also the full path towards that file.

A simple "MsgBox varFilename" iwll show that to you.

If it didn't contain the path, you wouldn't be able to open the file.

For the last 2 lines, you just need the filename without the path.

Set inputWorkbook = Workbooks(myFile)

will work, if myFile is a variable holding only the filename. It can be filled starting with varFilename, and working with functions such as Right, InStrRev and Long.

Since you are trying to learn VBA, it's a good exercise to do this yourself.

 

[RoryA]

Much simpler:

Sub OpenFile()
 
    Dim varFilename As String
    Dim inputWorkbook As Workbook
    varFilename = Application.GetOpenFilename("Excel Files, *.xl*") 'Get open file dialog, filter on files with *.xl*
    If varFilename = "False" Then Exit Sub 'If no file is selected, end the macro altogether
    Set inputWorkbook = Workbooks.Open(Filename:=varFilename)

End Sub

Note: don't ever use just 'End' in code.

 

[remmer75]

Much simpler:

Sub OpenFile()
 
    Dim varFilename As String
    Dim inputWorkbook As Workbook
    varFilename = Application.GetOpenFilename("Excel Files, *.xl*") 'Get open file dialog, filter on files with *.xl*
    If varFilename = "False" Then Exit Sub 'If no file is selected, end the macro altogether
    Set inputWorkbook = Workbooks.Open(Filename:=varFilename)
 
End Sub

Note: don't ever use just 'End' in code.

Thanks - works perfectly.

Why should I not use 'End' in code? Just using exit sub in this would let the rest of the macro (which isn't listed here) run with invalid data since the workbook would not be opened.

 

[RoryA]

Because it is roughly the code equivalent to closing Excel by terminating the excel.exe process in task manager. It will reset any global variables you have. And it's never necessary if you arrange your code correctly.

 

[remmer75]

Because it is roughly the code equivalent to closing Excel by terminating the excel.exe process in task manager. It will reset any global variables you have. And it's never necessary if you arrange your code correctly.

Thank you for taking time to explain the reason to me.

My macro has three subs, RunAll() (runs the next two in sequence), OpenFile() and CopyData() in the third sub it will copy a named range from the opened file to the file in which the macro resides (it's a reconciliation statement for the finance dep). But the second sub will fail if there is no named range in the file opened.

OpenFile() and CopyData() needs to be in that order, would you then suggest that I put a check into CopyData() and exit sub if varFilename:="False"? Then I'd put varfilename as a global scope variable instead of local.

Dim inputWorkbook As Workbook
Dim actWorkbook As Workbook
Dim BlueUpdated As Boolean
Dim WhiteUpdated As Boolean
 
Sub RunAll()
Dim ActSheet As Worksheet
Dim SelRange As Range
Dim i As Integer
Set actWorkbook = ActiveWorkbook
Set ActSheet = ActiveSheet
Set SelRange = Selection
BlueUpdated = False
WhiteUpdated = False
 
Application.ScreenUpdating = False
 
Do While Not (BlueUpdated = True And WhiteUpdated = True)
If i = 5 Then
MsgBox ("Exiting after five attempts")
Exit Sub
End If
OpenFile
CopyData
i = i + 1
Loop
 
actWorkbook.Activate
ActSheet.Select
SelRange.Select
Application.ScreenUpdating = True
MsgBox ("Workbook updated")
End Sub
 
Sub OpenFile()
 
Dim varFilename As String
varFilename = Application.GetOpenFilename("Excel Files, *.xl*") 'Get open file dialog, filter on files with *.xl*
If varFilename = "False" Then Exit Sub 'If no file is selected, end the macro altogether
Set inputWorkbook = Workbooks.Open(Filename:=varFilename)
End Sub
Sub CopyData()
' Determine if there is a named range called "WhiteCollarTotals" or "BlueCollarTotals"
' in the workbook just opened in OpenFile()
' if so, copy it to the file that opened the macro as values and formats
 
Dim varFileType As String
Dim outputtable As String
Dim inputtable As String
Dim inputrows As Integer
Dim inputcols As Integer
Dim outputrows As Integer
Dim outputcols As Integer
Dim inputrefer As String
Dim outputrefer As String
Dim inputsheet As String
Dim outputsheet As String
inputWorkbook.Activate
On Error Resume Next
If ActiveWorkbook.Names("BlueCollarTotals") Is Nothing Then
If ActiveWorkbook.Names("WhiteCollarTotals") Is Nothing Then
MsgBox ("No WhiteCollarTotals and" + vbLf + "No BlueCollarTotals" + vbLf + "Exiting")
Exit Sub 'If there is no proper range, exit
Else
varFileType = "WhiteCollar"
If WhiteUpdated = True Then
MsgBox ("White Collar already updated" + vbLf + "please select blue collar worksheet")
Exit Sub
End If
End If
Else
varFileType = "BlueCollar"
If BlueUpdated = True Then
MsgBox ("Blue Collar already updated" + vbLf + "please select white collar worksheet")
Exit Sub
End If
End If
On Error GoTo 0
 
outputtable = varFileType + "Table"
inputtable = varFileType + "Totals"
actWorkbook.Activate
outputrows = Range(outputtable).Rows.Count
outputcols = Range(outputtable).Columns.Count
outputrefer = Names(outputtable).RefersTo
outputsheet = Mid(outputrefer, 3, InStr(3, outputrefer, "'") - 3)
inputWorkbook.Activate
inputrows = Range(inputtable).Rows.Count
inputcols = Range(inputtable).Columns.Count
inputrefer = Names(inputtable).RefersTo
inputsheet = Mid(inputrefer, 3, InStr(3, inputrefer, "'") - 3)
If outputrows <> inputrows Then
MsgBox ("Different number of rows in input and output areas!" + vbLf + "Input: " + Str(inputrows) + " Def: " + inputrefer + vbLf + "Output: " + Str(outputrows) + " Def: " + outputrefer)
Exit Sub
ElseIf outputcols <> inputcols Then
MsgBox ("Different number of columns in input and output areas!" + vbLf + "Input: " + Str(inputcols) + " Def: " + inputrefer + vbLf + "Output: " + Str(outputcols) + " Def: " + outputrefer)
Exit Sub
End If
inputWorkbook.Activate
Sheets(inputsheet).Activate
Range(inputtable).Copy
actWorkbook.Activate
Sheets(outputsheet).Activate
Range(outputtable).Select
Selection.PasteSpecial Paste:=xlPasteValues
Selection.PasteSpecial Paste:=xlPasteFormats
Application.CutCopyMode = False
Range("A1").Select
If varFileType = "BlueCollar" Then
MsgBox ("Blue Collar Updated")
BlueUpdated = True
ElseIf varFileType = "WhiteCollar" Then
MsgBox ("White Collar Updated")
WhiteUpdated = True
End If
inputWorkbook.Close SaveChanges:=False
End Sub

Edit: Wow, that is not very readable - how does one preserve indentation from VBA when copying into code tags?

 

[RoryA]

The simplest solution is to have openfile run copydata at the end of its routine. You could also do what you suggested, or convert openfile to a function that returns a reference to the opened workbook or Nothing if it fails, then test the return before running copydata

 

[remmer75]

Simple and brilliant, thanks.