copy method affecting behaviour of seperate sub/module

Luke777

Board Regular
Joined
Aug 10, 2020
Messages
246
Office Version
  1. 365
Platform
  1. Windows
Hi all,

When using the following module (and different versions of - there's a lot of with statements in there as I've been troubleshooting so its a mess, sorry)...

VBA Code:
Sub ImportHSR()

    Dim wb1 As Workbook, wb2 As Workbook
    Dim ws1 As Worksheet, ws2 As Worksheet
    
    Set wb1 = Workbooks("workbook1.xlsm")
    Set wb2 = Workbooks.Open(FileName:="...")
    Set ws1 = wb1.Worksheets(7)
    Set ws2 = wb2.Worksheets("sheet1")
    
    With wb2
        ws2.Range(("A2:E2"), ws2.Range("A2:E2").End(xlDown)).Copy 
    End With
    With wb1
        ws1.Range("C2").PasteSpecial Paste:=xlPasteValues
        ws1.Columns("D:F").EntireColumn.Delete
    End With
    With wb2
        .Close
    End With
    
End Sub

the following code in a sub that runs a LOT later fails

VBA Code:
    Set ws1 = Worksheets(1)
        With ws1
            For Each Cell In ws1.Range(Cells(searchRow, 3), Cells(searchRow, lastCol)) **

the starred ** line specifically with the error message "method range of object worksheet failed"
However... If I change the sub doing the copy pasting to the folllowing
VBA Code:
Sub ImportHSR()

    Dim wb1 As Workbook, wb2 As Workbook
    Dim ws1 As Worksheet, ws2 As Worksheet
    
    Set wb1 = Workbooks("workbook1.xlsm")
    Set wb2 = Workbooks.Open(FileName:="...")
    Set ws1 = wb1.Worksheets(7)
    Set ws2 = wb2.Worksheets("sheet1")
    
    With wb2
        ws2.Range(("A2:E2"), ws2.Range("A2:E2").End(xlDown)).Copy Destination:=ws1.Range("C2")
        '.Close
    End With
    ws1.Range("C2").PasteSpecial Paste:=xlPasteValues
    
End Sub

everything works perfectly. Why is this? What's going on with that paste method that screws up some code over 7 subs later? also note that the line that fails works without "ws1." unless the copy sub also runs

I'm a bit baffled by this!

Thanks for any help!
 

Excel Facts

Does the VLOOKUP table have to be sorted?
No! when you are using an exact match, the VLOOKUP table can be in any order. Best-selling items at the top is actually the best.
I'm going to assume the searchRow or lastCol is the issue, the code snippet you provided does not indicate what these variables are associated to.
For Each Cell In ws1.Range(Cells(searchRow, 3), Cells(searchRow, lastCol)) **
 
Upvote 0
I'm going to assume the searchRow or lastCol is the issue, the code snippet you provided does not indicate what these variables are associated to.
Ah, should have included that but i still dont see why changing the copy method would affect this - also, looking at what it tries to do when it fails, it gets the right values for those two variables but tries to use them on the wrong sheet

VBA Code:
lastCol = ws1.Cells(1, Columns.Count).End(xlToLeft).Column

searchRow = Application.Match(psTime, rotaTimes, 1) + 3

ws1 = worksheets(1) on the sub with the error

long story short, searchRow = 5 and lastCol is 57, which is exactly what I'd expect because its getting those values from worksheets(1) its just trying to use them on the wrong sheet - so instead of looking at cells(5,3) on worksheets(1) its looking at cells(5,3) on worksheets(7) - worksheets(7) isn't mentioned at all in the sub with the error, but it IS the ws1 used in the sub with the copy paste
 
Upvote 0
Cells is like the single cell version of the range object. Both are context sensitive in that they are a subset of the object that precedes them either a Worksheet or a Range.
If you don't explicitly tell it what the parent is then it assumes the ActiveSheet.
So in your "For Each" line the Range is prefixed with "ws1" but not the Cells. It should look like this:
Rich (BB code):
For Each Cell In ws1.Range(ws1.Cells(searchRow, 3), ws1.Cells(searchRow, lastCol))

Since you already have the Set ws1 and With ws1 lines in your code.
You can then streamline the below (which includes my modification.)
Rich (BB code):
    Set ws1 = Worksheets(1)
        With ws1
            For Each Cell In ws1.Range(ws1.Cells(searchRow, 3), ws1.Cells(searchRow, lastCol))
                 ' Content of Loop
            Next Cell ' End of For Loop
         End With

To the this:
The ws1 is removed from before Range and before both Cells leaving just the full stop "." effectively being replaced by the With ws1 / End With statement that is wrapped around it.
Rich (BB code):
    Set ws1 = Worksheets(1)
        With ws1
            For Each Cell In .Range(.Cells(searchRow, 3), .Cells(searchRow, lastCol))
                 ' Content of Loop
            Next Cell ' End of For Loop   
        End With

You are also using the With / End With statements without actually using the functionality, in which case they don't really serve any purpose.
In another example, using
Rich (BB code):
    With wb1
        ws1.Range("C2").PasteSpecial Paste:=xlPasteValues
        ws1.Columns("D:F").EntireColumn.Delete
    End With

Here you already have ws1 defined as being in wb1
Rich (BB code):
Set ws1 = wb1.Worksheets(7)
so when you use ws1 you do not need to reference wb1.
The with encapsulation would be used for ws1 and your 4 lines above becomes this:
Rich (BB code):
    With ws1
        .Range("C2").PasteSpecial Paste:=xlPasteValues   ' ws1 removed from the front of ws1.Range
        .Columns("D:F").EntireColumn.Delete                   ' ws1 removed from the front of ws1.Columns
    End With

This is also makes the code run faster.
 
Upvote 0
Solution
Cells is like the single cell version of the range object. Both are context sensitive in that they are a subset of the object that precedes them either a Worksheet or a Range.
If you don't explicitly tell it what the parent is then it assumes the ActiveSheet.
So in your "For Each" line the Range is prefixed with "ws1" but not the Cells. It should look like this:
Rich (BB code):
For Each Cell In ws1.Range(ws1.Cells(searchRow, 3), ws1.Cells(searchRow, lastCol))

Since you already have the Set ws1 and With ws1 lines in your code.
You can then streamline the below (which includes my modification.)
Rich (BB code):
    Set ws1 = Worksheets(1)
        With ws1
            For Each Cell In ws1.Range(ws1.Cells(searchRow, 3), ws1.Cells(searchRow, lastCol))
                 ' Content of Loop
            Next Cell ' End of For Loop
         End With

To the this:
The ws1 is removed from before Range and before both Cells leaving just the full stop "." effectively being replaced by the With ws1 / End With statement that is wrapped around it.
Rich (BB code):
    Set ws1 = Worksheets(1)
        With ws1
            For Each Cell In .Range(.Cells(searchRow, 3), .Cells(searchRow, lastCol))
                 ' Content of Loop
            Next Cell ' End of For Loop 
        End With

You are also using the With / End With statements without actually using the functionality, in which case they don't really serve any purpose.
In another example, using
Rich (BB code):
    With wb1
        ws1.Range("C2").PasteSpecial Paste:=xlPasteValues
        ws1.Columns("D:F").EntireColumn.Delete
    End With

Here you already have ws1 defined as being in wb1
Rich (BB code):
Set ws1 = wb1.Worksheets(7)
so when you use ws1 you do not need to reference wb1.
The with encapsulation would be used for ws1 and your 4 lines above becomes this:
Rich (BB code):
    With ws1
        .Range("C2").PasteSpecial Paste:=xlPasteValues   ' ws1 removed from the front of ws1.Range
        .Columns("D:F").EntireColumn.Delete                   ' ws1 removed from the front of ws1.Columns
    End With

This is also makes the code run faster.
Thank you so much for what you’ve explained! I’ll be more conscious of how I use With statements now.

I’m still not too sure why it ran perfectly with one version of Sub ImportHSR but not the other. I’ll make adjustments following your guidance and see what happens.

I’m also thinking of switching to sheet codes instead of using dim worksheet and setting them - would this affect performance at all?
 
Upvote 0
I’m still not too sure why it ran perfectly with one version of Sub ImportHSR but not the other.
The most likely reason for it working is that the sheet you want to update, happens to be the ActiveSheet when that line of code runs.

I’m also thinking of switching to sheet codes instead of using dim worksheet and setting them - would this affect performance at all?

Do mean the Sheet code name ?
I don't tend to use it but I don't think it would affect performance by much if at all. If I had to guess, I would have thought it would be quicker but don't quote me on that.
Just keep in mind that codename only works if the sheet is in the same workbook as the macro.
 
Upvote 0

Forum statistics

Threads
1,224,802
Messages
6,181,054
Members
453,014
Latest member
Chris258

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top