Delete Row Based On Two Columns

K

Kris

Hello all and Happy 4th. I need some help if possible. Below is the
code I have found and tried to modify. What I am trying to accomplish
is if "47" is in column "L" AND "No" is in column "S" then that row is
deleted. Here is the code I tried to modify.

Sub Delete_rows_based_on_ColA_ColB()
Sheets("Working").Select
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Dim cell As Range, rng As Range, i As Long
Set rng = Columns("L").SpecialCells(xlConstants, xlTextValues)
For i = rng.Count To 1 Step -1
If LCase(rng(i).Value) = "47" _
And LCase(rng(i).Offset(7, 1).Value) = "No" _
Then rng(i).EntireRow.Delete
Next i
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub

Thank you,
Kris
 
G

Guest

1. Double check your data in Column L. Text, numeric, or both? Double check
rng.address in the immediate window while stepping through your code to
ensure SpecialCells is returning the correct range. You could use Union to
merge more than one range.
2. rng is probably is discontiguous range with multiple areas. Try stepping
through the areas first, then the rows in the area.
3. Your offset is not right. In VBA, it is (Row Offset, Column Offset). If
you want column S in the same row use Offset(0, 7).
4. Depending on if "47" could also be numeric, I would coerce to one
particular data type for comparison purposes (using CLng or CStr). VBA
didn't seem to mind, but I hate to depend on it.
5. Depending on what (if any) option compare statements you may be using,
"no" may not be the same as "No". If you use LCase to convert the data to
"no" you should use the same on the other side of your comparison.


Backup before trying.

Sub test()
Dim rng As Range
Dim i As Long, t As Long

On Error Resume Next
Set rng = Sheets("Working").Columns("L").SpecialCells(xlConstants,
xlTextValues)
If rng Is Nothing Then Exit Sub

On Error GoTo 0
For t = rng.Areas.Count To 1 Step -1
For i = rng.Areas(t).Rows.Count To 1 Step -1
If rng.Areas(t).Cells(i).Value = 47 _
And (rng.Areas(t).Cells(i).Offset(0, 7).Value) = "no" _
Then rng.Areas(t).Cells(i).EntireRow.Delete
Next i
Next t
End Sub
 
K

Kris

I can't get this code to run. I didn't mention that there is a header
row. Is this causing the problem since you mentioned VBA being picky
between numeric and text.

I also tried the uppercase lowercase for row S and each way it still
doesn't run.

Perhaps you could help me with this a little further.

Thank you,
Kris
 
G

Guest

Try this... It should be a bit faster than your existing code

Sub DeleteStuff()
Dim rngToSearch As Range
Dim rngFound As Range
Dim rngFoundAll As Range
Dim strFirstAddress As String
Dim wks As Worksheet

Set wks = ActiveSheet
Set rngToSearch = wks.Columns("S")
Set rngFound = rngToSearch.Find(What:="NO", _
LookIn:=xlValues, _
LookAt:=xlWhole, _
MatchCase:=False)
If Not rngFound Is Nothing Then
strFirstAddress = rngFound.Address
Do
If rngFound.Offset(0, -7).Text = "47" Then
If rngFoundAll Is Nothing Then
Set rngFoundAll = rngFound
Else
Set rngFoundAll = Union(rngFound, rngFoundAll)
End If
End If
Set rngFound = rngToSearch.FindNext(rngFound)
Loop Until rngFound.Address = strFirstAddress
If Not rngFoundAll Is Nothing Then rngFoundAll.EntireRow.Delete
End If
End Sub
 
G

Guest

I posted some small corrections below (with CStr and LCase functions). My
last response initially got lost w/ "Server Busy" error so I had to retype it
and forgot I had made some small changes.

Also, I need to correct myself, you could not use CLng as your text 47's
could be mixed with other text data (such as "Bob") and VBA can't convert
that to numeric. Although VBA seems to treat "47" the same as 47. I try to
be consistent because Excel functions are picky about it, as is SpecialCells.

What error are you getting? Or, is the code running but not deleting
anything? The fact that it does not run does not help. This ng usually
wraps text in unexpected places. The Set rng statement s/b all one line, but
I know when the response is posted it will get wrapped to another line. If
the code runs, but doesn't do anything, You can add watches (Debug/Add Watch)
to watch certain values as you step through the code (using F8 key).

Sub test()
Dim rng As Range
Dim i As Long, t As Long

Application.ScreenUpdating = False

On Error Resume Next
Set rng = Sheets("Working").Columns("L").SpecialCells(xlConstants,
xlTextValues)
If rng Is Nothing Then Exit Sub

On Error GoTo 0
For t = rng.Areas.Count To 1 Step -1
For i = rng.Areas(t).Rows.Count To 1 Step -1
If CStr(rng.Areas(t).Cells(i).Value) = "47" _
And LCase(rng.Areas(t).Cells(i).Offset(0, 7).Value) = "no" _
Then rng.Areas(t).Cells(i).EntireRow.Delete
Next i
Next t

Application.ScreenUpdating = True

End Sub
 
K

Kris

Sorry. Let me be more specific. It is not deleting anything. The data
shows up as 47 and No. I stepped through the macro and the second row
has the data that should cause a deletion and nothing happens. I know
the code is running but it is not deleting anything.

Any ideas?
 
A

Andrew Taylor

The condition

LCase(rng(i).Offset(7, 1).Value) = "No"

will never be satisfied - you need

LCase(rng(i).Offset(7, 1).Value) = "no"

Andrew
 
K

Kris

Ok I have figured this out and boy do I feel dumb. When my company
automaticaly generates the data I am using it shows up as "_No_" with
the underline indicating spaces. I added the two spaces and wouldn't
you know this script works perfect.


Thank you so much for the help.

Kris King
 
G

Guest

I probably should have used Trim to get rid of potential extra spaces:
And LCase(Trim(rng.Areas(t).Cells(i).Offset(0, 7).Value)) = "no" _

Also, as Jim pointed out, it will be slow if you have a large amount of
data. If you need to speed up the process, you should take a look at Jim's
macro. You could also use a for each loop instead of the nested For/Next
loops - which would eliminate the issue of the number of areas (I should have
done it this way in the first place, but I got caught up with correcting the
existing code instead of using a better solution). Also, like Jim's macro,
instead of deleting each line one at a time, all of the lines meeting the
criteria are deleted once, which I believe will improve the speed.


Sub test2()
Dim rngData As Range
Dim rngCell As Range
Dim rngDelete As Range

Application.ScreenUpdating = False
Set rngData = Sheets("Working").Columns("L").SpecialCells(xlConstants,
xlTextValues)

For Each rngCell In rngData
If CStr(rngCell.Value) = "47" _
And LCase(Trim(rngCell.Offset(0, 7).Value)) = "no" Then
If rngDelete Is Nothing Then
Set rngDelete = rngCell
Else: Set rngDelete = Union(rngDelete, rngCell)
End If
End If
Next rngCell

If Not rngDelete Is Nothing Then _
rngDelete.EntireRow.Delete

Application.ScreenUpdating = True

End Sub
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top