Need to specify worksheet for column insertion and population

I'm building VBA inside Access to modify an Excel file.  I have two fields on my form (txtFileNa and cmbWorksheet) that contain the path&file ("C:\Spreadsheets\Problem.xlsx") and the worksheet name ("ThisSpreadsheet").  

 I need to specify a worksheet for inserting and populating a new column.  The code below works fine as long as I'm putting the column in the first worksheet in the workbook, but if I need to put the column in the 4th worksheet (named "ThisSpreadsheet"), it still goes in the first worksheet.  I've tried these changes on line 90:

   90    Set xlSheet = xlBook.Worksheets(4)
   90    Set xlSheet = xlBook.Worksheets("ThisSpreadsheet")
   90    Set xlSheet = xlBook.Worksheets(me.cmbWooksheet)

neither of them work.  I think I need to modify lines 100 and 110 to specify the worksheet, but I don't know how.  I'd appreciate any suggestions for making the code better, in addition to fixing the specification issue.  Thanks!


Dim xl As Excel.Application, xlBook As Excel.Workbook, xlSheet As Excel.Worksheet, filePath As String
     
60    filePath = Me.txtFileNa
70    Set xl = New Excel.Application
80    Set xlBook = xl.Workbooks.Open(filePath)
90    Set xlSheet = xlBook.Worksheets(1)

           
'add new column to spreadsheet
100   Range("A1").EntireColumn.Insert

'put description in header row:
110   Range("$A$1").Value = "SpreadsheetRowID"

'populate the cells with sequential numbers as long as the rows have data
    Dim k, i As Long, n As Long
120       With Range("a2:a" & Range("b" & Rows.Count).End(xlUp).row)
130     k = .Value
140     For i = 1 To UBound(k, 1)
150         If Len(k(i, 1)) = 0 Then
160             n = n + 1
170             k(i, 1) = n
180         End If
190     Next
200     .Value = k
210       End With


    'Save
220       xlBook.Save
230       xlBook.Close
240       xl.Quit
250       xl.Application.Quit
260       Set xl = Nothing
270       Set xlSheet = Nothing
LVL 8
Paul Cook-GilesSenior Application DeveloperAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

NorieAnalyst Assistant Commented:
Lines 100 and 110 don't refer to any worksheet so they will affect the active sheet.

You also need worksheet references in the subsequent code]
Dim k As Long, i As Long, n As Long

' rest of declarations and code.

    Set xlSheet = xlBook.Worksheets("ThisSpreadsheet")
    
    With xlSheet
        .Range("A1").EntireColumn.Insert
        .Range("$A$1").Value = "SpreadsheetRowID"

       With .Range("a2:a" & Range("b" & .Rows.Count).End(xlUp).Row)
            k = .Value
            For i = 1 To UBound(k, 1)
                If Len(k(i, 1)) = 0 Then
                    n = n + 1
                    k(i, 1) = n
                End If
            Next
            .Value = k
        End With
        
    End With

Open in new window


PS You should always make sure everything is referenced properly especially when automating Excel from another application.
Dale FyeOwner, Developing Solutions LLCCommented:
I generally include a combo box with a ValueList row source type on my forms that interact with Excel.  After I select the Excel file to load, I open the file and then loop through the Sheets collection to identify the names of each of the worksheets in the workbook.  I append those value to the combo box using the Add method.

This gives me the ability to select which worksheet I want to work with.  Then I would have a "cmd_Export" button or something like that which selects the appropriate worksheet and performs the code you have above.  This requires that you define your Excel objects in the forms declaration section, not in a specific event procedure so that xl and xlBook can be defined at one step and xlSheet can be defined in a 2nd step.
Gustav BrockCIOCommented:
Yes, if you work a lot with a range, dim and specify it:

Dim rng As Range
Set rng = xlsheet.Range(<specification of range>)

/gustav
Protecting & Securing Your Critical Data

Considering 93 percent of companies file for bankruptcy within 12 months of a disaster that blocked access to their data for 10 days or more, planning for the worst is just smart business. Learn how Acronis Backup integrates security at every stage

Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Thank you, Norie!
I now have this:

Dim k As Long, i As Long, n As Long

60    filePath = Me.txtFileNa
70    Set xl = New Excel.Application
80    Set xlBook = xl.Workbooks.Open(filePath)
90    Set xlSheet = xlBook.Worksheets(Me.cmbWorkBookName)

100       With xlSheet
110     .Range("A1").EntireColumn.Insert
120     .Range("$A$1").Value = "SpreadsheetRowID"
130       With .Range("a2:a" & Range("b" & Rows.Count).End(xlUp).Row)
140            k = .Value
150               For i = 1 To UBound(k, 1)
160               If Len(k(i, 1)) = 0 Then
170                  n = n + 1
180                  k(i, 1) = n
190               End If
200            Next
210         .Value = k
220       End With
230     End With


When I run it, I get "Compile Error:  Expected array", and Ubound(k, is highlighted on line 150.
NorieAnalyst Assistant Commented:
That's my fault.

I assumes k was one of the loop variables so added a declaration of Long for it

To fix Just remove that.
Dim k , i As Long, n As Long

Open in new window

Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Norrie, thank you;  my revised code is below. I'm still getting an error from line 100, where I need to define which worksheet should be active.  I can paste in an index number (and it seems to work), but if I put in the reference to the field where the name of the worksheet is selected (me.cmbWorksheetname) it chokes.  

Using the index number I got the code to work once.  The second time, I got a 462 error on line 140:  "The Remote server machine does not exist or is unavailable."  Googling that error returned

https://anictteacher.files.wordpress.com/2011/11/vba-error-462-explained-and-resolved.pdf

It seems to me that lines 250 - 300 would address the issue the article raises... what am I missing?

Paul

Dim k, i As Long, n As Long

70    filePath = Me.txtFileNa
80    Set xl = New Excel.Application
90    Set xlBook = xl.Workbooks.Open(filePath)
100   Set xlSheet = xlBook.Worksheets(1)

110       With xlSheet
120     .Range("A1").EntireColumn.Insert
130     .Range("$A$1").Value = "SpreadsheetRowID"
140       With .Range("a2:a" & Range("b" & Rows.Count).End(xlUp).Row)
150            k = .Value
160               For i = 1 To UBound(k, 1)
170               If Len(k(i, 1)) = 0 Then
180                  n = n + 1
190                  k(i, 1) = n
200               End If
210            Next
220         .Value = k
230       End With
240     End With


    'Save
250       xlBook.Save
260       xlBook.Close
270       xl.Quit
280      xl.Application.Quit

290       Set xl = Nothing
300       Set xlSheet = Nothing
NorieAnalyst Assistant Commented:
How does it 'choke'?

Do you get any error message(s)?

Have you checked that a sheet with the value from the field exists in the workbook you are opening?

What type of field/control are you getting the sheet name from?

PS When developing code that automates another application it's a good idea to make the application visible.

In your code you can do that right after you've created the new instance of Excel.
Set xl = New Excel.Application
xl.Visible = True

Open in new window

Gustav BrockCIOCommented:
You may need a "$" for the name:

WorksheetName = Me!cmbWorksheetname.Value & "$"


>      With .Range("a2:a" & Range("b" & Rows.Count).End(xlUp).Row)

Again, do dim and specify explicitely your ranges. "Range" is flying in the air:

 Dim rngSource As Range
 Dim rngTarget As Range
 Set rngSource = xlsheet.Range(<specification of range>)
 Set rngTarget = xlsheet.Range(<specification of range>)

It has nothing to do if the worksheet is active and indeed not visible. These settings are only interesting if the happenings should be followed by a user. If not, it only complicates matters and slows down processing.

Also, be very specific cleaning up when you close down. All objects in reverse order as opened, or your application will be hanging in the background:

    'Save

xlBook.Save
Set <any range> = Nothing
Set xlSheet = Nothing
xlBook.Close
Set xlBook = Nothing
xl.Quit
Set xl = Nothing

 /gustav
NorieAnalyst Assistant Commented:
Paul

Just realised there's a tiny mistake,  a missing dot qualifier.
With .Range("a2:a" & .Range("b" & Rows.Count).End(xlUp).Row)

Open in new window

That dot is needed to 'tie' the range back to the sheet in the previous With statement.

PS You can close and save in one go.
xlBook.Close SaveChanges:=True

Open in new window


Gustav

There's no need for a $, that's only needed when querying a sheet in SQL.
Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Norie and Gustav, thanks very much for your patience and persistence!

a) I don't want to make the app visible, as this process will be used by non-tech-friendly folks, and I don't want to freak them out with a flashing screen.  <G>
b) Gustav, I'm not sure where in the code below I'd add the Range and Target statements;  where should I put them, and what (exactly) should they say?
c) Access didn't like the Range close statement (line 255 below);  do I have the syntax wrong?
d) I added the missing dot qualifier before running the code this morning, and it ran  without a hiccup.  :) THANK YOU NORIE!!  I haven't tried running it a second time, as I've frequently had problems with the app not releasing/closing/nothing-ing in previous attempts.  I added the Shell line (#310), which seems to help;  is that Bad Practice?
e) what should "k" be dimensioned as?

Paul  

Dim k, i As Long, n As Long

60    filePath = Me.txtFileNa
70    Set xl = New Excel.Application
80    Set xlBook = xl.Workbooks.Open(filePath)
90    Set xlSheet = xlBook.Worksheets(1)

100       With xlSheet
110     .Range("A1").EntireColumn.Insert
120     .Range("$A$1").Value = "SpreadsheetRowID"
130       With .Range("a2:a" & .Range("b" & Rows.Count).End(xlUp).Row)
140            k = .Value
150               For i = 1 To UBound(k, 1)
160               If Len(k(i, 1)) = 0 Then
170                  n = n + 1
180                  k(i, 1) = n
190               End If
200            Next
210         .Value = k
220       End With
230     End With


    'Save and close

240   xlBook.Save
250       xlBook.Close
'255   Set .Range = Nothing
260       Set xlSheet = Nothing
270    Set xlBook = Nothing
280       xl.Quit
290      xl.Application.Quit
300       Set xl = Nothing

310   Shell ("taskkill /f /im excel.exe")
Gustav BrockCIOCommented:
Well, you still miss a lot of details. Read carefully again.

Thus:

Dim rng1 As Range
Dim rng2 As Range
etc.

 60    filePath = Me.txtFileNa
 70    Set xl = New Excel.Application
 80    Set xlBook = xl.Workbooks.Open(filePath)
 90    Set xlSheet = xlBook.Worksheets(1)

Set rng1 = xlSheet.Range("A1")
set rng2 = xlSheet.Range("$A$1")
' Other ranges.

rng1.EntireColumn.Insert
rng2.Value = "SpreadsheetRowID"

' Do stuff to/from/with ranges.

' Save and close

   xlBook.Save
'   xlBook.Close     ' Not here. Objects are still alive.
   Set rng1 = Nothing
   Set rng2 = Nothing
' etc.
   Set xlSheet = Nothing
   xlBook.Close
   Set xlBook = Nothing
   xl.Quit
   Set xl = Nothing

' Not needed if you do it right.
' Shell ("taskkill /f /im excel.exe")

/gustav
NorieAnalyst Assistant Commented:
Paul

a) I'm only suggesting you make it visible while you are developing the code. Having the app visible means you can see what's going on in it which is quite handy when debugging.

 Once you've got the code up and running you can remove the code to make it visible.

b) You don't really need the range objects, the code is fine as it is.

c) You can't close a range and I don't think that's been suggested.

d) I don't see any reason why the code as it is now would leave ghost instances of Excel.
 
    Everything, eg ranges, worksheets etc, appears to be qualified properly and you seem to be closing down/disposing    of things properly.
   
    If you are finding Excel isn't closing properly then having it visible might help finding out why that is.

e)  k should be declared as Variant, and if you don't specify a data type when declaring a variable it defaults to Variant. So you don't need to change anything.

By the way, what is the code meant to do anyway?

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Dale FyeOwner, Developing Solutions LLCCommented:
When automating Excel, you must be very careful to release the Excel objects in the proper order.  Doing it incorrectly or in the wrong sequence can leave ghost instances of Excel in the task manager, bloating your memory usage.

When I want to leave Excel open for the user to view or use, I use the following code:

    If Not xlsht Is Nothing Then Set xlsht = Nothing
    If Not xlwbk Is Nothing Then Set xlwbk = Nothing
    If Not xl Is Nothing Then Set xl = Nothing

But if I want to close the active workbook and Excel, I use code similar to that posted by Gustav.  I put this code in the exit portion of my code modules and ensure that the error handler resumes at the ProcExit if an error is encountered, something like:
Private Sub SomeProcedure

    On Error goto ProcError

    'insert some code here

ProcExit:
    On Error Resume next
    If Not sht Is Nothing Then Set sht = Nothing
    If Not wbk Is Nothing Then Set wbk = Nothing
    If Not xl Is Nothing Then Set xl = Nothing
    Exit Sub

ProcError:
    'code to handle errors here
    Resume ProcExit

End Sub

Open in new window

This general syntax ensures that even if an error is encountered, that the Excel objects will be released properly.
Gustav BrockCIOCommented:
Thanks Dale.

/gustav
Dale FyeOwner, Developing Solutions LLCCommented:
G, you sure were busy here yesterday!
Gustav BrockCIOCommented:
Rey and Scott had an offday.

/gustav
Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Norie, this process (inserting a new column, and populating it with sequential numbers for all rows with data) preceeds the importation of the spreadsheets data into an Access db for further manipulation.  The data --unfortunately-- does not arrive with a unique key, and we don't assign unique keys until after the manipulation is done.  Part of the manipulation is replacing single multi-month rows with one row for each month, and I need to be able to identify the source row in the archived spreadsheet.

Thank you very much for your help!

Paul
Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Norie, it looks like one of the bugs is back;  I get "Error 13: type mismatch" on line 150.
Also, I need to modify line 90 so I can use "(me.cmbWorksheetName)" in place of (1), so I can import worksheets other than the first one in the workbook.

Dim k As Variant, i As Long, n As Long

60    filePath = Me.txtFileNa
70    Set xl = New Excel.Application
80    Set xlBook = xl.Workbooks.Open(filePath)
90    Set xlSheet = xlBook.Worksheets(1)

100       With xlSheet
110     .Range("A1").EntireColumn.Insert
120     .Range("$A$1").Value = "SpreadsheetRowID"
130       With .Range("a2:a" & .Range("b" & Rows.Count).End(xlUp).Row)
140            k = .Value
150               For i = 1 To UBound(k, 1)
160               If Len(k(i, 1)) = 0 Then
170                  n = n + 1
180                  k(i, 1) = n
190               End If
200            Next
210         .Value = k
220       End With
230     End With

'Save and close
240   xlBook.Save
250   xlBook.Close
260   Set xlSheet = Nothing
270   Set xlBook = Nothing
280   xl.Quit
290   xl.Application.Quit
300   Set xl = Nothing

310   Shell ("taskkill /f /im excel.exe")
Gustav BrockCIOCommented:
If you still need this line:

    Shell ("taskkill /f /im excel.exe")

you still haven't got it right.

/gustav
NorieAnalyst Assistant Commented:
When you get the type mismatch how many rows of data are on the worksheet the code is referring to?

(To find that out you could make Excel visible and step through the code with F8)

As for the sheet name coming from a control/field I'll go back to my original question.

Have you checked that a sheet with the value from the field exists in the workbook you are opening?
Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Thanks, Gustav-- I'll comment out the line and see if anything adverse happens.  :)

Norie:  Yes, I've checked;  the combo box is populated with the list of worksheet names in the workbook:

Private Sub PopulateWorkBookNames(ByVal pWorkBook As String)
      '---------------------------------------------------------------------------------------
      ' Date      : 3/2/2015
      ' Purpose   : drops list of WorkBook names into WorkBook combo box
      '              code based on http://stackoverflow.com/questions/18412697/read-excel-file-sheet-names
      '---------------------------------------------------------------------------------------
10    On Error GoTo HandleError

Dim objExc As Object, objWbk As Object, objWsh As Object, strValueList As String, strFirstWorkbook As String

20        Set objExc = CreateObject("Excel.Application")
30        Set objWbk = objExc.Workbooks.Open(pWorkBook)
         
'Set objExc = New Excel.Application
40        Set objWbk = objExc.Workbooks.Open(pWorkBook)
50        For Each objWsh In objWbk.Worksheets
60        strValueList = strValueList & objWsh.Name & "; "
70    Next

'populate cmbWorkBook
80    DoCmd.GoToControl "cmbWorkSheetName"
90    strFirstWorkbook = FirstBit(strValueList, ";")
100   Me.cmbWorkSheetName.Text = strFirstWorkbook
110   Me.cmbWorkSheetName.RowSource = strValueList

120       Set objWsh = Nothing
130       objWbk.Close
140       Set objWbk = Nothing
150       objExc.Quit
160       Set objExc = Nothing
         
ExitSub:
170      Exit Sub

HandleError:
180   If Err.Number = 1004 Then
   'do nothing;  this is the error returned when the FilePicker dialog box, invoked by GrabFileName,  is cancelled.
190      Exit Sub
200   End If

210      MsgBox "Error " & Err.Number & ": " & Err.Description & vbCrLf & _
              "Line:  " & Erl & vbCrLf & _
              "Sub 'PopulateWorkBookNames'  in VBA Document 'Form_ImportSpreadsheetFrm'", vbOKOnly, strAppNa
220      Resume ExitSub
EndSub:
End Sub
NorieAnalyst Assistant Commented:
Are you sure that the value in the code is the name of a worksheet in the workbook you are opening?

You can check that by stepping through the code with F8 and seeing what value is coming from the combobox.

Also, if you make Excel visible you can check the sheet names of the workbook you've opened.

PS Why are you using late-binding in the code you just posted and early binding in the original code you posted?
Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Yes, I'm sure;  I'm using F8 to step through the code.

i've got no idea of the difference between late-binding and early-binding;  this is code I've found online and adapted just enough for it to reliably do what I want it to do.  :)
Gustav BrockCIOCommented:
Originally you use early binding:

  Set xl = New Excel.Application

However, once objects are declared and assigned, the remaining code is identical.

/gustav
Paul Cook-GilesSenior Application DeveloperAuthor Commented:
Gustav, is one or the other (early vs late) Best Practice?    

And I'm seeing that some examples have created and dimensioned  the variables like this:
Dim xl As Excel.Application, xlBook As Excel.Workbook, xlSheet As Excel.Worksheet, filePath As String
70    Set xl = New Excel.Application
80    Set xlBook = xl.Workbooks.Open(filePath)
90    Set xlSheet = xlBook.Worksheets(1)

and some like this:
Dim objExc As Object, objWbk As Object, objWsh As Object, strValueList As String, strFirstWorkbook As String
20        Set objExc = CreateObject("Excel.Application")
30        Set objWbk = objExc.Workbooks.Open(pWorkBook)          
35        Set objExc = New Excel.Application
40        Set objWbk = objExc.Workbooks.Open(pWorkBook)
50        For Each objWsh In objWbk.Worksheets

Is there a good template for this you can recommend?  I've been working in VBA in Access for years, but I've only lately started it with Excel, and if there's a good introduction/survey post about it somewhere, I'd love to read it.

Paul
Gustav BrockCIOCommented:
I prefer early binding if at all possible. But you will find others preferring late binding. A matter of habit and preference.

/gustav
NorieAnalyst Assistant Commented:
If the code of going to be distributed then late binding is probably the way to go.

The reason for that is late binding eliminates the need for the user to set a reference to a specific version of an object library.

However after saying that I would recommend using early binding when developing

The reason for that is that it will allow you the advantage of Intellisense.
NorieAnalyst Assistant Commented:
* of = is

Seriously, this phone needs bigger buttons or I need thinner fingers.
Paul Cook-GilesSenior Application DeveloperAuthor Commented:
OK, I resolved the 'open a worksheet other than the first one' issue;  I added Index to the properties I was collecting (strValueList = strValueList & objWsh.Name & "; " & objWsh.Index & "; ") and used the Index value to define the worksheet I wanted to import (see code below.)  I tried using   Me.cmbWorkSheetName.Column(1) in line 100, but got an out-of-range message.  Using the variable to hand off the value works, though.

However.... I'm still getting error 462 (The remote server machine does not exist or is unavailable." from line 140.

Dim k As Variant, i As Long, n As Long

60    filePath = Me.txtFileNa
70    Set xl = New Excel.Application
80    Set xlBook = xl.Workbooks.Open(filePath)
90    intWorkSheetIndex = Me.cmbWorkSheetName.Column(1)
100   Set xlSheet = xlBook.Worksheets(intWorkSheetIndex)  'value here has to be the index number, not the name, of the worksheet!

110       With xlSheet
120     .Range("A1").EntireColumn.Insert
130     .Range("$A$1").Value = "SpreadsheetRowID"
140       With .Range("a2:a" & .Range("b" & Rows.Count).End(xlUp).Row)
150            k = .Value
160               For i = 1 To UBound(k, 1)
170               If Len(k(i, 1)) = 0 Then
180                  n = n + 1
190                  k(i, 1) = n
200               End If
210            Next
220         .Value = k
230       End With
240     End With

'Save and close
250   xlBook.Save
260   xlBook.Close
270   Set xlSheet = Nothing
280   Set xlBook = Nothing
290   xl.Quit
300   xl.Application.Quit
310   Set xl = Nothing
Gustav BrockCIOCommented:
It would probably help debugging if you defined the ranges one by one explicitly.

/gustav
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Microsoft Access

From novice to tech pro — start learning today.