Troubleshooting Excel VBA Code

Question

The point of this code is to take user inputs from a "Remove Flags" tab in which the user puts an item number and what program it belongs to, filters the "Master List" tab by the item number and the program, then match the name of the flag to the column and delete the flag. However the offset is not working. It is instead deleting the header. When I step through it everything works fine until the line I marked with '*******.

I am fairly new to VBA and am self taught so any and all help is greatly appreciated. Thank you very much for your time.

EDIT: Removed "On Error Resume Next" and fixed some spelling errors. Current issue is with rng not having >1 rows when it is filtered and definitely has two rows (one row is the header, one row is the returned data.)

Sub RemoveFlag()
Dim cel As Range
Dim rg As Range
Dim d As Double
Dim i As Integer
Dim m As Integer
Dim n As Integer
Dim rng As Range
Dim wsMaster As Worksheet
Dim wsFlag As Worksheet
Set wsMaster = Worksheets("Master List")
Set wsFlag = Worksheets("Remove Flags")
i = 6

'If there is no data. Do nothing.
If wsFlag.Range("C6") = "" Then
    wsFlag.Activate
Else
    Application.ScreenUpdating = False

'Add Leading zeroes
wsFlag.Activate
Set rg = Range("C6")
Set rg = Range(rg, rg.Worksheet.Cells(Rows.Count, rg.Column).End(xlUp))
rg.NumberFormat = "@"
For Each cel In rg.Cells
If IsNumeric(cel.Value) Then
    d = Val(cel.Value)
    cel.Value = Format(d, "000000000000000000") 'Eighteen digit number
End If
Next

'Clear all the filters on the Master List tab.
    wsMaster.Activate
If wsMaster.AutoFilterMode = True Then
    wsMaster.AutoFilterMode = False
End If

'Loop through all lines of data
    Do While wsFlag.Cells(i, 3).Value <> ""
'Filter by the SKU number
wsMaster.Range("A1").AutoFilter Field:=4, Criteria1:=wsFlag.Cells(i, 3).Value
'Filter by the Program
    wsMaster.Range("A1").AutoFilter Field:=2, Criteria1:=wsFlag.Cells(i, 2).Value
'If the filter is not empty find the column of the flag
Set rng = wsMaster.UsedRange.SpecialCells(xlCellTypeVisible)

If (rng.Rows.Count > 1) Then
    wsMaster.Range("A1:Z1").Find(wsFlag.Cells(i, 4), LookIn:=xlValues).Activate
    n = ActiveCell.Column
    Sheets("Master List").Range.Offset(1, 0).SpecialCells(xlCellTypeVisible).Select
    m = ActiveCell.Row
    Cells(m, n) = ""
    wsFlag.Activate
    wsFlag.Range(Cells(i, 2), Cells(i, 4)).ClearContents
Else
    wsFlag.Activate
    wsFlag.Range(Cells(i, 2), Cells(i, 4)).Copy
    wsFlag.Range("F4").End(xlDown).Offset(1, 0).Activate
    ActiveCell.PasteSpecial Paste:=xlPasteValues
    wsFlag.Range(Cells(i, 2), Cells(i, 4)).ClearContents
End If
    wsMaster.Activate
    wsMaster.AutoFilterMode = False
i = i + 1
Loop

'Make sure the entire Master List tab is not highlighted and pull the 'highlighted cell' to A1 in both tabs.
    wsMaster.Activate
    wsMaster.Range("A1").Activate

wsFlag.Activate
Range("A1").Activate

'Unfreeze the screen
Application.ScreenUpdating = True

End If
End Sub

What is the issue you are having? What error message do you get? Which line(s) does the issue occure on? — Mr.Burns, Feb 16 '17 at 15:16
The issue is that it is not clearing the proper cell in the Master List. It is instead deleting the header. It isn't giving me an error, just performing the wrong task. I think the issue is with the definition of "m" because it is setting m=1 instead of whatever would be appropriate (145 in my test case as the number I want to remove the flag from is in the 145 row of data). The filters apply correctly and it finds the correct column, it just clears the wrong cell. Line is denoted with '*********** — Emily Alden, Feb 16 '17 at 15:19
Do you have sheets called `"Master List"`, `"MasterList"` and `"Maser List"`? — Darren Bartrup-Cook, Feb 16 '17 at 15:19
Your code references all those sheet names - I'm guessing you have one called `Master List` and the others are typos. Also, does your `FIND` line actually find anything? If not your `ActiveCell` won't move and it `n` will be whatever row is active before the find command. — Darren Bartrup-Cook, Feb 16 '17 at 15:22
This line "activesheet.Range.Offset(1, 0).SpecialCells(xlCellTypeVisible).Select" resolves into an error and is ignored, so you don't offset down off the header row and set m to that row, hence clearing the header. You left "on error resume next" on so wasn't made aware of the error, it's bad practice and leads to problems like this. — Zerk, Feb 16 '17 at 15:24
Hadn't spotted the `On Error Resume Next`, that'll cover a multitude of sins. +1 — Darren Bartrup-Cook, Feb 16 '17 at 15:25
@DarrenBartrup-Cook it will be masking the incorrect worksheet references also. — Zerk, Feb 16 '17 at 15:26
@DarrenBartrup-Cook: Is there a fast way to check for those spelling errors (like a tool) or are you just that good at spot checking? I believe the find is working correctly because it deletes in the correct column. — Emily Alden, Feb 16 '17 at 15:27
@Zerk I've removed the "On Error Resume Next" and will keep that in mind for the future. I had it in there so that if the user input a non-numeric text at the beginning it wouldn't throw up a 'scary' warning. . Do you know why that line is resolving in an error? — Emily Alden, Feb 16 '17 at 15:29
[How to avoid using Select and Activate in Excel VBA macros](http://stackoverflow.com/q/10714251/1188513) — Mathieu Guindon, Feb 16 '17 at 15:30
Just good at spot checking. :) But, if you go to `View ~ Toolbars ~ Customize`. Then click on the `Commands` tab and the `Debug` category and drag the `Compile Project` icon onto your toolbar (I put it next to the Save button). Press this before saving and it will highlight any commands it can't compile. Helps a bit. — Darren Bartrup-Cook, Feb 16 '17 at 15:34
@EmilyAlden good practice is to set a worksheet object and reference that, assuming you use capitals it's quick and easy to see where a reference is incorrect. These two lines would help a lot. Dim wsMaster as worksheet, set wsMaster = worksheets("Master List"). You can then reference it as wsMaster.range(x,y) — Zerk, Feb 16 '17 at 15:37
I mispelled "Criteria1:=" as "Criteria1" on the second filter. So the filter was getting skipped. Now It's returning an "Else" on "If (rng.Rows>1)"... So something about my definition of visible cells must be off. — Emily Alden, Feb 16 '17 at 15:37
@Zerk Sure. Could you explain the difference between a lower case "count" and an uppercase "Count" in the excel VBA language? (Google is terrible at capitalisation specific questions.) ((Sorry on my post I said "rng.Rows", but in the actual code it is "rng.Rows.Count" sorry for the confusion)) — Emily Alden, Feb 16 '17 at 15:44
@GordonBell I'll work on that now. There are a lot of 'best practice' aspects that I seem to have completely missed. I do appreciate all this help! ((Would those be As Variant or As String ?)) — Emily Alden, Feb 16 '17 at 15:45
@EmilyAlden No difference, Excel would autocorrect lazy typed lower case to "proper" case, it often makes it an advantage to type in lower case as you know there's an issue if it isn't properly corrected and capitalized. — Zerk, Feb 16 '17 at 15:51
Comment out the ScreenUpdating property changes while debugging, it can help you see what's happening. And you can easily insert an Exit Sub in the middle of your code to see progress to that point. Like do that, and insert a n Exit Sub right after the Filter. Is what you expect filtered? — Gordon Bell, Feb 16 '17 at 16:00
@GordonBell Yes. It ends up with one row of data which is the row I expect. (I usually just use the 'step into ability', but the Exit Sub is good to know). — Emily Alden, Feb 16 '17 at 16:39
@GordonBell Now I'm getting a "Wrong number of arguments" for the line Sheets("Master List").Range.Offset(1, 0).SpecialCells(xlCellTypeVisible).Select — Emily Alden, Feb 16 '17 at 18:31

score 2 · Answer 1 · answered Feb 16 '17 at 15:50

2

As @Zerk suggested, first set two Worksheet variables at top of code:

Dim wsMaster As Worksheet
Dim wsRemoveFlags As Worksheet

Set wsMaster = Worksheets("Master List")
Set wsRemoveFlags = Worksheets("Remove Flags")

Then replace all other instances of Worksheets("Master List") with wsMaster and Worksheets("Remove Flags") with wsRemoveFlags.

answered Feb 16 '17 at 15:50

Gordon Bell

13,337
3
45
64

I appreciate this suggestion and agree that it cleans up the code significantly, but since the code is still not functioning as intended I'm not going to accept this as the solution. – Emily Alden Feb 16 '17 at 17:04

Gordon Bell · Accepted Answer · 2017-02-16T20:08:52.610

Sometimes it's easier to just loop through your rows and columns. Something like the following:

Replace everything between:

Do While wsFlag.Cells(i, 3).Value <> ""
   ...
Loop

with:

Do While wsFlag.Cells(i, 3).Value <> "" 
    Dim r As Long  ' Rows
    Dim c As Long  ' Columns
    Dim lastRow As Long
    Dim found As Boolean

    lastRow = wsMaster.Cells.SpecialCells(xlLastCell).Row
    found = False

    For r = 2 To lastRow ' Skipping Header Row
        ' Find Matching Program/SKU
        If wsMaster.Cells(r, 2).Value = wsFlag.Cells(i, 2).Value _
        And wsMaster.Cells(r, 3) = wsFlag.Cells(i, 3).Value Then
            ' Find Flag in Row
            For c = 1 To 26   ' Columns A to Z
                 If wsMaster.Cells(r, c) = wsFlag.Cells(i, 4) Then
                     ' Found Flag
                     wsMaster.Cells(r, c) = ""
                     found = True
                     Exit For ' if flag can be in more than one column, remove this.
                 End If
            Next 'c
        End If
    Next 'r

    If Not found Then
        ' Here is where you need to put code if Flag wsFlag.Cells(i, 4) not found.
    End If
Loop

Troubleshooting Excel VBA Code

2 Answers2