Why doesn't this code work?

spoogehead

New Member
Joined
Sep 15, 2002
Messages
31
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!
 

Excel Facts

Copy a format multiple times
Select a formatted range. Double-click the Format Painter (left side of Home tab). You can paste formatting multiple times. Esc to stop

Joe4

MrExcel MVP, Junior Admin
Joined
Aug 1, 2002
Messages
58,378
Office Version
  1. 365
Platform
  1. Windows
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.
Code:
Do Until Cells(i, 1).Value = "End of list" 
   ...
   i=i+1
Loop
 

spoogehead

New Member
Joined
Sep 15, 2002
Messages
31
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

MrExcel MVP, Junior Admin
Joined
Aug 1, 2002
Messages
58,378
Office Version
  1. 365
Platform
  1. Windows
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

New Member
Joined
Sep 15, 2002
Messages
31

ADVERTISEMENT

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

Well-known Member
Joined
Apr 28, 2004
Messages
76,300
Office Version
  1. 365
Platform
  1. Windows
Try this.
Code:
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
 

Forum statistics

Threads
1,136,991
Messages
5,679,014
Members
419,799
Latest member
APInfa

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Top