Why doesn't this code work?

[spoogehead]

Sub Macro1()
Dim defender As Integer 'defender is a number (Count number of DEF)
Dim i As Integer 'i is a number (row selection)
i = 1
Dim cell As Range
Application.ScreenUpdating = False

Range("A30").Value = "End of list"

Do

defender = 0
Cells(i, 1).Select
If "Pi" = DEF Then defender = defender + 1
If "Qi" = DEF Then defender = defender + 1
If "Ri" = DEF Then defender = defender + 1
If "Si" = DEF Then defender = defender + 1
If "Ti" = DEF Then defender = defender + 1
If "Ui" = DEF Then defender = defender + 1
If "Vi" = DEF Then defender = defender + 1
If "Wi" = DEF Then defender = defender + 1
If "Xi" = DEF Then defender = defender + 1
If "Yi" = DEF Then defender = defender + 1
If "Zi" = DEF Then defender = defender + 1

If defender > 4 Then
Rows("i:1").Select
Selection.Delete Shift:=xlUp

If defender = 4 Then i = i + 1

Loop Until Cells(i, 1).Value = "End of list"
End Sub

When I execute this macro I get a Loop without Do error but the Do command is there? (I know the code is a bit crude but I'm still learning the basics at the moment). Thanks for any assistance!

 

[Joe4]

I see a bunch of problems. Here are the obvious problems I see.

1. What is DEF? Is it a variable or a named range? If it is the value "DEF", it needs to be enclosed in quotes.

2. Rows("i:1").Select is wrong
Should it be Rows("1:" & i).Select or Rows(i).Select depending on what you are after

3. Your loop is wrong. It needs both a start and end, and you need to increment the value of i within the loop itself, or else it will never change and you may get caught in an infinite loop.

Do Until Cells(i, 1).Value = "End of list" 
   ...
   i=i+1
Loop

 

[spoogehead]

Thanks for the reply!

1. DEF is simply short for defender - what I am trying to do is look for the word DEF in the cell range Pi to Zi where i is the row number (starting at row 1). Should DEF be in quotes?

2. If the number of DEF's found = 4 then i is incremented by 1 and the DEF check begins for the next row.

3. If number of DEF's is <> 4 (forgot to put the < in my original post) then the current row indicated by i should be deleted. When I recorded the macro for selecting and deleting row 1, the VBA code in the macro was:
Rows("1:1").Select
Selection.Delete Shift:=xlUp

Should the code read: Rows("i").Select
Selection.Delete Shift:=xlUp ???

4. I altered the loop as you suggested but am still getting Loop without Do error??

 

[Joe4]

Yes. All literal text values must be enclosed in quotes. Otherwise, it is treated like a variable.

But I still don't understand you statements:
If "Pi" = DEF Then defender = defender + 1
specifically "Pi" and DEF. Can you give more details with exactly what this is doing (maybe provide an example, or post a section of the spreadsheet)?

When deleting rows, it is always best to work backwards, or else you risk skipping rows if you have consecutive rows that are supposed to be deleted.

Rather than us trying to figure out what the code is supposed to be doing, it may make more sense to post a section of your data, and give a detailed description of what you are trying to do.

 

[spoogehead]

Row 1, cells P1 to Z1 contains the following (for example):

GLK DEF DEF DEF DEF DEF DEF MID MID STR STR

Row 2, cells P2 to Z2 contain the following (for example):

GLK DEF DEF DEF DEF MID MID MID MID MID STR

and so on, potentially for thousands of rows, each row may have different occurrances of DEF, MID, STR etc

My code is trying (hopefully) to locate all the instance of DEF, 1 row at a time beginning at row 1. In this example, row 1 contains 6 instances of DEF. My condition says that any total of DEF which does NOT equal 4 then delete that row. So in this instance I want to delete row 1 (and then shift up the next row so that gaps aren't left).

We then look at row 2 (although due to shift up in this code this would now be called row 1). There are 4 instances of DEF here - this is a valid row so I want to leave the entire row as it stands, and select the next row (row 2 but in reality row 3)

And so on......Eventually I want to do this for instances of MID and STR but I was trying to get DEF right first

 

[Norie]

Try this.

Sub DeleteDEFS()
Dim rng As Range
Dim LastRow As Long
Dim I As Long

    LastRow = Range("P" & Rows.Count).End(xlUp).Row
    
    For I = LastRow To 1 Step -1
        Set rng = Range("P" & I).Resize(, 11)
        If Application.WorksheetFunction.CountIf(rng, "DEF") > 4 Then
            rng.EntireRow.Delete
        End If
    Next I
    
End Sub

 

[spoogehead]

Works a treat! Thanks Norie!