nbozzy
asked on
VBA code in Excel only works when stepped through line by line!
I have VBA code that uses the "Activeworkbook.RefreshAll " command to update a table whose data comes from our AS400. It refreshes with an SQL query.
All the rest of my code after the refresh works fine EXCEPT a line where I'm trying to find the last row of data from the AS400. The code line is: bot = wsInstall.Cells(Cells.Rows .Count, "G").End(xlUp).Row
It won't update unless I break the code after that line, then drag the yellow arrow back up to that row and step into it again. Help!! I'm going nuts!
All the rest of my code after the refresh works fine EXCEPT a line where I'm trying to find the last row of data from the AS400. The code line is: bot = wsInstall.Cells(Cells.Rows
It won't update unless I break the code after that line, then drag the yellow arrow back up to that row and step into it again. Help!! I'm going nuts!
A sample file, or a transcript of your code would help. Otherwise, the only option I see that could cause that behaviour is that line being inside an IF or CASE clause and being skipped over. When you break it, you then force the code inside the clause, ignoring the conditions.
Have you tried putting in Debug.Print lines before and after the offending line?
Debug.Print bot
bot = wsInstall.Cells(Cells.Rows .Count,"G" ).End(xlUp ).Row
Debug.Print bot
I have had instances where the code "gets ahead" and simply using debug.print slows it down enough to get it to work correctly.
Also, Cells is a property of a Range object. Try using a Range object in leiu of Cells.
bot = wsInstall.Range("G" & Rows.Count).End(xlUp).Row
Hopefully one of those works for you.
Cheers,
WC
Debug.Print bot
bot = wsInstall.Cells(Cells.Rows
Debug.Print bot
I have had instances where the code "gets ahead" and simply using debug.print slows it down enough to get it to work correctly.
Also, Cells is a property of a Range object. Try using a Range object in leiu of Cells.
bot = wsInstall.Range("G" & Rows.Count).End(xlUp).Row
Hopefully one of those works for you.
Cheers,
WC
SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
ASKER
Ok, I'll post code .... WarCrimes' suggestions didn't work. and Cluskitt's suggestion gave really weird results: when I mouse over the variable name, it says "bot = 8" but when I mouse over the phrase "wsInstall.UsedRange.Rows. Count," it gives me the correct answer of "bot = 86."
Row 8 is the first row of data from the query; row 86 is the last.
Row 8 is the first row of data from the query; row 86 is the last.
Sub EquipCklist()
'Creates or re-creates Install Checklist tab for users
Application.ScreenUpdating = False
'On Error Resume Next
'run Function InvalidSWTotal and exit if bad line totals are found
Worksheets("Equipment Worksheet").Activate
If InvalidSWTotal = True Then
Application.ScreenUpdating = True
Exit Sub
End If
Dim wb As Workbook
Dim wsEWork As Worksheet 'Equipment Worksheet sheet where x's will appear
Dim wsInstall As Worksheet 'Install Checklist sheet
Dim wsSW As Worksheet 'SW Worksheet
Dim rddy As String
Dim lrow As Integer
Dim bot As Integer
Dim rng As Range
Dim item As Range
Dim nextECopy As Integer
Dim currRow As Integer
Dim nix As Range
Dim hgt As String
Dim clr As String
Dim startNon As Integer
Dim rowQ As Integer
Dim adRows As Range
Dim r As Long
ActiveWorkbook.Unprotect Password:="3253"
'check to see if Install Checklist is hidden
If Worksheets("Install Checklist").Visible = xlSheetHidden Then
Worksheets("Install Checklist").Visible = xlSheetVisible
ElseIf Worksheets("Install Checklist").Visible = xlSheetVisible Then
'delete existing data
Worksheets("Install Checklist").Activate
Worksheets("Install Checklist").Range("G9:J2000").ClearContents
End If
'run Billy's link to Movex (pulls Equipment List by CO# - limit 3)
'first, a warning
rddy = MsgBox("To create the Installation Checklist, you must have" & vbCrLf _
& "the Royston CO number(s) ready." & vbCrLf _
& "You will be asked for THREE CO numbers. If you only have one, " & vbCrLf _
& "type it in all THREE boxes.", vbOKCancel, "Create Installation Checklist")
If rddy = vbCancel Then
'they bailed out
MsgBox "You clicked the Cancel button. The Installation Checklist will not be created."
ActiveWorkbook.Protect Password:="3253", Structure:=True
Application.ScreenUpdating = True
Exit Sub
End If
Worksheets("Install Checklist").Activate
'now force run of Billy's link to Movex
Application.ScreenUpdating = True
ActiveWorkbook.RefreshAll
Application.ScreenUpdating = False
MsgBox "Equipment List was gathered from Movex."
'set up
Set wb = ActiveWorkbook
Set wsEWork = wb.Sheets("Equipment Worksheet")
lrow = 390
Set wsInstall = wb.Sheets("Install Checklist")
Set wsSW = wb.Sheets("SW Worksheet")
wsInstall.Activate
Rows("7:7").Select
Selection.EntireRow.Hidden = True
wsInstall.Range("J2:J4").Select
wsInstall.Range("G4").Activate
Debug.Print bot
bot = wsInstall.UsedRange.Rows.Count
'bot = wsInstall.Range("G" & Rows.Count).End(xlUp).row
Debug.Print bot
'make column L set for double-click X
Range("L8:L" & bot).Select
With Selection
.Font.name = "Marlett"
.Font.Bold = True
.Font.color = RGB(255, 0, 0) 'red
.Interior.color = RGB(255, 255, 0) 'yellow
End With
SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
I still suggest you turn off the background query.
Curious, what did my suggestion return as a result for bot?
In order to use my suggestion, you have to use the End(xlUp) on a Column you know will have data in the last row. If column G doesn't have data in the last row, just switch the code to one that does.
In order to use my suggestion, you have to use the End(xlUp) on a Column you know will have data in the last row. If column G doesn't have data in the last row, just switch the code to one that does.
Rory,
Nice one. Finally gave in and Googled this. Looks like your suggestion is the answer.
nbozzy,
You can do it manually as Rory has suggested, or with code. Instead of
ActiveWorkbook.RefreshAll
use
wsInstall.QueryTables(1).R efresh BackgroundQuery:=False
assuming there is only one query table on the sheet.
That should make the VBA wait for the refresh to complete and the rest of the code to work as expected.
No points for me. All to Rory for this one.
WC
Nice one. Finally gave in and Googled this. Looks like your suggestion is the answer.
nbozzy,
You can do it manually as Rory has suggested, or with code. Instead of
ActiveWorkbook.RefreshAll
use
wsInstall.QueryTables(1).R
assuming there is only one query table on the sheet.
That should make the VBA wait for the refresh to complete and the rest of the code to work as expected.
No points for me. All to Rory for this one.
WC
ASKER
WarCrimes (and Rory): I get "subscript out of range" when using the manual refresh code line you provided above.
WarCrimes: row wasn't capitalized because it should have been "rowS." Silly me. And today (before switching to the manual refresh code), I got really weird results: bot was set to 273 when it should have been 86. Next run it set to 127 (I think) when it should have been 86. Strange.
WarCrimes: row wasn't capitalized because it should have been "rowS." Silly me. And today (before switching to the manual refresh code), I got really weird results: bot was set to 273 when it should have been 86. Next run it set to 127 (I think) when it should have been 86. Strange.
Try changing the property manually and then you can carry on using your RefreshAll code.
ASKER
Rorya, what property? Do you mean that I should deselect background refresh in the connection properties box?
Yes.
ASKER
That did it, Rorya!!!!!!!!!!!!!!!!!! <nbozzy does a cartwheel>
This has been an incredible bear ... thanks so much to all. Rorya, I'm indebted to you!
WarCrimes, thanks VERY much for the suggestions about best practices. I LOVE those!
And Cluskitt, thanks for introducing me to UsedRange.
This has been an incredible bear ... thanks so much to all. Rorya, I'm indebted to you!
WarCrimes, thanks VERY much for the suggestions about best practices. I LOVE those!
And Cluskitt, thanks for introducing me to UsedRange.
Since you obviously benefitted from the others' responses, I think a points split would be fairer?
As for me, I don't need the points. Besides, my suggestion might have been useful, but it didn't answer the question.
Nobody needs the points (in spite of what some may seem to think), but I think it would be fair here.
ASKER
Ok, but how do I go back in and modify points?
I didn't mean need with that meaning. I meant, I don't think they should be split with me, for the above mentioned reasons. :)
To modify the points, hit the request attention button on top and ask for the question to be reopened and the points reassigned.
I always think that if your answer was of value to the Asker, then they should be free to give you a share of the points - assuming they think it's fair too, of course! ;)
nbozzy,
Just to point out, in this line, which ended up having nothing to do with your issue,
'bot = wsInstall.Range("G" & Rows.Count).End(xlUp).row
It is supposed to be .Row, because you only want to return the value of the row of the last used cell in column G. I use this all the time for that purpose. The reason you were getting odd results is because the VBA wasn't waiting for the query to refresh, hence it was "getting ahead".
As for the "Subscript Out of Range" you get this error when an object you are referencing doesn't exist,
wsInstall.QueryTables(1).R efresh BackgroundQuery:=False
is before this line in your code
Set wsSW = wb.Sheets("SW Worksheet")
So it doesn't know what wsInstall is yet. If you move that line up to set the object before using it, it will get rid of the error.
Glad you got your solution, and no need to split points. There are plenty to go around.
Cheers,
WC
Just to point out, in this line, which ended up having nothing to do with your issue,
'bot = wsInstall.Range("G" & Rows.Count).End(xlUp).row
It is supposed to be .Row, because you only want to return the value of the row of the last used cell in column G. I use this all the time for that purpose. The reason you were getting odd results is because the VBA wasn't waiting for the query to refresh, hence it was "getting ahead".
As for the "Subscript Out of Range" you get this error when an object you are referencing doesn't exist,
wsInstall.QueryTables(1).R
is before this line in your code
Set wsSW = wb.Sheets("SW Worksheet")
So it doesn't know what wsInstall is yet. If you move that line up to set the object before using it, it will get rid of the error.
Glad you got your solution, and no need to split points. There are plenty to go around.
Cheers,
WC
Sorry, somehow I copied the wrong line. The line you need to move up is
Set wsInstall = wb.Sheets("Install Checklist")
Set wsInstall = wb.Sheets("Install Checklist")
Actually, because you are using wb in that line you need to move other lines up as well.
Since you said you love best practices, let me give you another one.
If you have static objects that won't change throughout your code, try to put their Set statements before any code. This ensures they will exist when they are needed. Sometimes it is necessary to reassign values during execution, but in your code I would recommend pulling these lines down
Application.ScreenUpdating = True
ActiveWorkbook.RefreshAll
Application.ScreenUpdating = False
to after this line
Set wsSW = wb.Sheets("SW Worksheet")
I know it wasn't your original design so maybe you just didn't realize it. It can be tricky when piecing together code from other sources. Again, glad it is working. Signing off from this thread.
WC
Since you said you love best practices, let me give you another one.
If you have static objects that won't change throughout your code, try to put their Set statements before any code. This ensures they will exist when they are needed. Sometimes it is necessary to reassign values during execution, but in your code I would recommend pulling these lines down
Application.ScreenUpdating
ActiveWorkbook.RefreshAll
Application.ScreenUpdating
to after this line
Set wsSW = wb.Sheets("SW Worksheet")
I know it wasn't your original design so maybe you just didn't realize it. It can be tricky when piecing together code from other sources. Again, glad it is working. Signing off from this thread.
WC
Let me point out that I hate that I cannot edit a post and am forced to add another comment to the chain all the time. I know why this is the case, but it is frustrating when you want to add something to your post.
Obviously after moving those 3 lines down, change the RefreshAll to the line that just refreshes the single query table that I supplied yesterday. Then it should work, no error.
Seriously, last comment. Got work to do.
WC
Obviously after moving those 3 lines down, change the RefreshAll to the line that just refreshes the single query table that I supplied yesterday. Then it should work, no error.
Seriously, last comment. Got work to do.
WC
ASKER
WarCrimes, the first time I got the "subscript out of range" error, I did take all those "set" statements and move them up. As a rule, I do those right after the Dim's but this time I thought I should reserve all the possible PC memory for the refresh (our PC's are not overburdened with RAM). But with me, it's always best to err on the side of GIVING me information rather than assuming, because I'm self-taught and have no co-workers that code.
And it's funny, but the code window won't capitalize the word Row ... only RowS with an S. Wonder why?
And it's funny, but the code window won't capitalize the word Row ... only RowS with an S. Wonder why?
I'm guessing you use row as a variable name somewhere (not a good idea).
ASKER
No, rorya, I didn't. But I did use: lrow, currRow, rowQ, adRows, and r. Think those are problems?
No those are fine, but if row is not being capitalised I'd put money on the fact that you have used row for something else somewhere (doesn't have to be in the same sub necessarily)
It could be a named range.
ASKER
Nope. Not either. I did take enough programming classes to know not to use a word like that. But just to be sure, I checked the "Name Manager." Also tried Row singular again, and the editor removes the capital letter R if the word doesn't have an "S" at the end. Where else can I check?........you know? Something must be up...I just looked at a Worksheet_BeforeDoubleClic k event, and in all the code, references to "Target.Row" got changed to lower-case "r" for row. But the code works.
If it removes the capital R, then it assumes that row is a reserved name for something. Else it would just leave it as it was. Try a Debug.Print row.
ASKER
That gave me a "variable not defined" error.
That is a strange occurrance, but if you're sure the code is working fine as is, then I wouldn't worry too much about it. I remember in some access front end I made once there was some variable that it would capitalize though I hadn't declared it anywhere. I had it declared once somewhere, but had since deleted it.
In the end, if things work fine, you can let it go, though I prefer to fix those little mysteries. If you need to change your code, it may stop working and then you have to find a fix urgently, instead of taking your time to do it while it isn't necessary. At the very least, you might learn something from it :)
In the end, if things work fine, you can let it go, though I prefer to fix those little mysteries. If you need to change your code, it may stop working and then you have to find a fix urgently, instead of taking your time to do it while it isn't necessary. At the very least, you might learn something from it :)
ASKER
Thanks, Cluskitt!
Glad to help :)
ASKER
Thank you ALL very much. I wish I could spread more than 500 points around!