Link to home
Start Free TrialLog in
Avatar of nbozzy
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!
Avatar of Cluskitt
Cluskitt
Flag of Portugal image

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.
Avatar of Cory Vandenberg
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
SOLUTION
Avatar of Cluskitt
Cluskitt
Flag of Portugal image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of nbozzy
nbozzy

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.
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

Open in new window

SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
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.
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).Refresh 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
Avatar of nbozzy

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.
Try changing the property manually and then you can carry on using your RefreshAll code.
Avatar of nbozzy

ASKER

Rorya, what property? Do you mean that I should deselect background refresh in the connection properties box?
Avatar of nbozzy

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.
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.
Avatar of nbozzy

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).Refresh 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
Sorry, somehow I copied the wrong line.  The line you need to move up is

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
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
Avatar of nbozzy

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?
I'm guessing you use row as a variable name somewhere (not a good idea).
Avatar of nbozzy

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.
Avatar of nbozzy

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_BeforeDoubleClick 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.
Avatar of nbozzy

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 :)
Avatar of nbozzy

ASKER

Thanks, Cluskitt!
Glad to help :)
Avatar of nbozzy

ASKER

Thank you ALL very much. I wish I could spread more than 500 points around!