Why does an Excel VBA routine not work if it is called by another routine?

Question

I have a Sub that I Call in as part of another sub. When this is ran by itself it works fine but when it is called in from another sub it doesn't quite work exactly right. The main sub saves the default values and then:

Application.ScreenUpdating = False
Application.DisplayStatusBar = False
Application.Calculation = xlCalculationManual
Application.EnableEvents = False
ActiveSheet.DisplayPageBreaks = False

Call FillInCoding

Here is the sub that is not working and I think it might be this coding is copy and paste special-ing:

Sub FillInCoding()

'clears the area to be pasted into
Worksheets(1).Range("A11:G98567").ClearContents


Dim JE As Worksheet
Dim Sheet1 As Worksheet
Dim lastRow As Long

Set JE = ThisWorkbook.Worksheets(1)
Set Sheet1 = ThisWorkbook.Worksheets(2)


lastRow = Sheet1.Range("R65536").End(xlUp).Row

'This part is copying the values of the ranges from worksheet #2 into the worksheet #1 destination
 Worksheets(2).Range("AM2:AM" & lastRow).Copy
 Worksheets(1).Range("A11:A" & lastRow).PasteSpecial xlPasteValues
 Worksheets(2).Range("AN2:AN" & lastRow).Copy
 Worksheets(1).Range("B11:B" & lastRow).PasteSpecial xlPasteValues
 Worksheets(2).Range("R2:R" & lastRow).Copy
 Worksheets(1).Range("E11:E" & lastRow).PasteSpecial xlPasteValues
 Worksheets(2).Range("AO2:AO" & lastRow).Copy
 Worksheets(1).Range("G11:G" & lastRow).PasteSpecial xlPasteValues


End Sub

I tried setting the ranges but it was pasting the cell formulas instead of the values, so this was the only easy way I could get it to do what I needed it to do. At the end of the day I need what you see on sheet 2 in a varying range (each time it's ran it could be a different length) to be copied as a value into sheet 1.

If someone knows how to easily set ranges on different sheets as equal to varying values when the end row changes and they are in different spots on the different sheets that would be ideal.

For example on sheet two it might be

A1="XYZ"
B1="100000"
C1="52.00"
D1="office supplies"

A2="YZA"
B2="150000"
C2="-52.00"
D2="office supplies"

But in sheet one we need these values pasted in starting at A11:12, B11:12, C11:12, D11:12, etc (the next day though maybe there are 40 lines instead of 2 lines).

Answer 1

I appreciate the answers given, I learned something from all of them. What I ended up needing to do to get the macro to run and copy the data over exactly right was:

calcState = Application.Calculation
'when this was turned off before the next coding it didn't copy over the company name properly (column A)
Application.Calculation = calcState

'Puts the coding into the JE Sheet
Call FillInCoding

'turning it back off to speed up the final bits of coding
Application.Calculation = xlCalculationManual

I'm not really sure why having Calculation mode as Manual instead of Automatic makes it so that sometimes the columns weren't copying over correctly in the example I listed in column A instead of copying (from the second sheet to the first) over something like

A1 "XYZ"
A2 "YZA"
A3 "GHI"
A4 "XYZ"

it was putting:

A1 "XYZ"
A2 "XYZ"
A3 "XYZ"
A4 "XYZ"

Sorry I think I wasn't the clearest that is what was happening, my coding was working overall by itself but @Jeeped's is clearly cleaner, simpler, and just better. I'm still fairly new to coding generally and this site.

I got the idea on turning those options off of automatic to manual from https://blogs.office.com/2009/03/12/excel-vba-performance-coding-best-practices/ It does make the parent macro significantly faster to do as is suggested there, but I guess it can cause issues.

Answer 2

There are a couple different problems that can occur due to the way your code is written.

First, you're copying a range that is larger than the area you are attempting to paste it in. In your exacmple:

 Worksheets(2).Range("AM2:AM" & lastRow).Copy
 Worksheets(1).Range("A11:A" & lastRow).PasteSpecial xlPasteValues

If your lastRow variable has 100 entries, then you just copied 99 cells (2-100). Yet you're trying to paste them in a space that's only 90 cells (11-100). One bad, yet really simple, unsophisticated fix is to simply add the difference to the lastrow variable:

 Worksheets(2).Range("AM2:AM" & lastRow).Copy
 Worksheets(1).Range("A11:A" & lastRow **+ 9**).PasteSpecial xlPasteValues

But better yet, you don't need to explicitly state the range you'll be pasting into. You can just give the target cell and it'll paste there regardless of the content length:

Worksheets(2).Range("AM2:AM" & lastRow).Copy
Worksheets(1).Range("A11").PasteSpecial xlPasteValues

Also, there's a few other things you're doing that may cause you some issues later on. you're setting your lastRow range to the length of column R on your first spreadsheet. When based on how you are copying and pasting, I'm guessing you want your lastRow length to be the length of the column you're actively copying. You can get that by using something like:

lastRow = Sheets("Sheet2").Range("A" & Rows.Count).End(xlUp).Row

and change "A" to whatever your target column is. Then reset the last row variable each time you're copying a new row. An Example would be:

 lastRow = Sheets("Sheet2").Range("AM" & Rows.Count).End(xlUp).Row
 Sheets("Sheet2").Range("AM2:AM" & lastRow).Copy
 Sheets("Sheet1").Range("A11").PasteSpecial xlPasteValues

 lastRow = Sheets("Sheet2").Range("AN" & Rows.Count).End(xlUp).Row
 Sheets("Sheet2").Range("AN2:AN" & lastRow).Copy
 Sheets("Sheet1").Range("B11").PasteSpecial xlPasteValues

Lastly, i changed Worksheets(1) to calling the sheet by name, because I find this method is more intuitive and allows you to identify which sheets you're copying and pasting from, even after you or a user has shifted the page order around.

So your final code will look like the below. There are definitely cleaner ways to write all this (including turning a lot of the repeated code, like sheets into variables), but I think this is the logical next step from the code you're currently writing:

Dim LastRow as long

     lastRow = Sheets("Sheet2").Range("AM" & Rows.Count).End(xlUp).Row
     Sheets("Sheet2").Range("AM2:AM" & lastRow).Copy
     Sheets("Sheet1").Range("A11").PasteSpecial xlPasteValues

     lastRow = Sheets("Sheet2").Range("AN" & Rows.Count).End(xlUp).Row
     Sheets("Sheet2").Range("AN2:AN" & lastRow).Copy
     Sheets("Sheet1").Range("B11").PasteSpecial xlPasteValues

     lastRow = Sheets("Sheet2").Range("R" & Rows.Count).End(xlUp).Row
     Sheets("Sheet2").Range("R2:R" & lastRow).Copy
     Sheets("Sheet1").Range("E11").PasteSpecial xlPasteValues

     lastRow = Sheets("Sheet2").Range("A0" & Rows.Count).End(xlUp).Row
     Sheets("Sheet2").Range("A02:A0" & lastRow).Copy
     Sheets("Sheet1").Range("G11").PasteSpecial xlPasteValues

Answer 3

It is probably not a good idea to be using a worksheet's reserved .CodeName property as the name of a variable; particlarly so as it isn't even that worksheet's codename.

Sub FillInCoding()
    Dim lr As Long, JE As Worksheet, ws2 As Worksheet

    Set JE = ThisWorkbook.Worksheets(1)
    Set ws2 = ThisWorkbook.Worksheets(2)

    lr = ws2.Range("R65536").End(xlUp).Row

    With JE
        .Range("A11:G98567").ClearContents

        .Range("A11:B11").Resize(lr - 1, 2) = ws2.Range("AM2:AN" & lr).Value
        .Range("E11").Resize(lr - 1, 1) = ws2.Range("R2:R" & lr).Value
        .Range("G11").Resize(lr - 1, 1) = ws2.Range("AO2:AO" & lr).Value
    End With

End Sub

I've opted for direct value transfer using some modified worksheet references. The first two columns AM & AN to A and B could be doubled up. See my comment about setting the last row.

See How to avoid using Select in Excel VBA macros for more methods on getting away from relying on select and activate to accomplish your goals.

Answer 4

One thing to note is that you can skip the copy/paste part, and just set two range values equal to eachother:

Worksheets(2).Range("AM2:AM" & lastRow).Copy
 Worksheets(1).Range("A11:A" & lastRow).PasteSpecial xlPasteValues

is the same as

Worksheets(1).Range("A11:A" & lastRow).Value = Worksheets(2).Range("AM2:AM" & lastRow).Value

As for working with different sheets, I highly recommend creating two variables to hold these, and work directly (and explicitly) with each sheet:

Sub test()
Dim ws1 As Worksheet, ws2 As Worksheet

' Let's set "Sheet1" to be ws1, and "Sheet2" to be ws2
Set ws1 = Worksheets("Sheet1")
Set ws2 = Worksheets("Sheet2")

'Now, use WITH to work in a specific sheet.
With ws1
    .Range("A1").Value = "Cell A1" ' Note the . at the beginning of Range().  This makes sure that the range you're using is
    ' on sheet1, not any other sheet

    'The next line is the SAME as using Range("A1"), only using Cells(row,Column). NOTE the . before Range() AND Cells()
    .Range(.Cells(1, 1), .Cells(1, 1)).Value = "Cell A1"
End With

With ws2
    .Range("A1").Value = "Sheet2, Cell A1"
End With

'Let's now say you want sheet1, A1 to be put in Sheet2, A1:

' [copy TO range] = [where you want to copy FROM]
ws2.Range("A1").Value = ws1.Range("A1").Value

'Or, of course, we can use ranges
ws2.Range("A1:B100").Value = ws1.Range("A1:B100").Value
' is same as
ws2.Range(ws2.Cells(1, 1), ws2.Cells(100, 2)).Value = ws1.Range(ws1.Cells(1, 1), ws1.Cells(100, 2)).Value
' is same as
With ws2
    .Range(.Cells(1, 1), .Cells(100, 2)).Value = ws1.Range(ws1.Cells(1, 1), ws1.Cells(100, 2)).Value
End With

End Sub

Hopefully the above is clear, and you can see how to edit your macro to make sure to pull data and paste data to the right pages.

Edit: Andddd I just noticed you did set some worksheets already - so just use the above with those. After re-reading, I think I explained your question (or at least part of it)...

Edit2: Wow, okay so I completely misunderstood your question. Sorry about that! I figure I will leave this here though as it does explain setting values, no copy/paste. Sorry mate, I'll have another look at your question. (If someone more familiar with StackOverflow can let me know, should I keep this answer here, or just delete completely? What's the forum guideline?)