0

I am looking for some help regarding my For Each loop in Excel VBA.

Set ListofCells = Range(ActiveCell, Range("C12:C9999").Find("!!End Measures!!"))

For Each SingleCell In ListofCells

   If ActiveCell.Value = "!!End Measures!!" Then
        ActiveCell.Offset(1, 0).Select
        Exit For
    ElseIf ActiveCell.Value = "" Then
        ActiveCell.EntireRow.Delete
    End If
Next SingleCell

For some reason this likes to exit the loop early for, as far as I can tell, absolutely no reason. In my spreadsheet I'm looking to delete any unused rows from the ActiveCell down to where the entry-area ends, which I have denoted as "!!End Measures!!". I have also tried: 

Set ListofCells = Range(ActiveCell, ActiveCell.End(xlDown))

and it as the same behavior. Ending the loops many rows earlier than expected.

If I put 4 of these loops in a row then it works as intended....but I'm hoping to find out why this might be exiting early instead of having to do something weird like looping this 4 or 5 times.

Any help is greatly appreciated and if I can answer any questions or provide any more information I'd be happy to.

Ryan Shinnick
  • 15
  • 1
  • 7
  • somewhere in your cells, there's something that's triggering one of your end measures. that's the only reason the if statement would trigger an exit. so, either your end measures are coded incorrectly, or you're interpreting your data wrong. – acousticismX Nov 16 '17 at 21:02
  • I agree that's the only reason I can think of that would end it, but my column from ActiveCell all the way down to the cell with !!End Measures!! in it is completely empty, satisfying the ActiveCell.Value = "" clause. I just don't get it.... – Ryan Shinnick Nov 16 '17 at 21:04
  • are you sure you're defining your ListofCells correctly? read this link https://msdn.microsoft.com/en-us/vba/excel-vba/articles/range-range-property-excel and then follow up to see if your syntax is correct. I suspect it may not be – acousticismX Nov 16 '17 at 21:11
  • Test if the cells you believe are empty, actually are empty. In an other column put "=if (c12 = "", "", "XXX")" and copy down to see if they really are empty. There an many reasons why a cell looks empty but isn't. – mooseman Nov 16 '17 at 21:13
  • Thanks for the test mooseman. I did that and copied down; unfortunately the cells in the C column are indeed blank and did not get a "XXX". And acousticismX, I double checked and I think I am doing it correctly. I am using the same syntax in multiple other places in this macro and it's working correctly there. – Ryan Shinnick Nov 16 '17 at 21:20
  • As soon as your code hits a non-blank cell (other than the `"!!End Measures!!"` one), it will just do nothing more until it does hit the `"!!End Measures!!"` one because `ActiveCell` will always be that first non-blank cell. – YowE3K Nov 16 '17 at 21:28
  • Yow, that's the problem. I ONLY have blank cells from ActiveCell down to "!!End Measures!!" https://imgur.com/a/PiQgF This is a screenshot of my column AFTER it leaves the loop. The cells are all completely blank, but it exits the loop anyways. – Ryan Shinnick Nov 16 '17 at 21:32
  • If you know all the cells are blank, why do a loop? Why not just delete all those rows in one hit - i.e. `Range(ActiveCell, Range("C12:C9999").Find("!!End Measures!!").Offset(-1, 0)).EntireRow.Delete` – YowE3K Nov 16 '17 at 21:37
  • If you want to see why it is only deleting half the rows, add a `Debug.Print ListOfCells.Address & "," & SingleCell.Address` just after the `For Each SingleCell In ListofCells` statement. Each time you delete a row, you reduce the size of the range you are acting on, and each iteration you are looking at one cell lower in that (reduced) range. – YowE3K Nov 16 '17 at 21:42
  • Yow, I swear.....I tried exactly the same syntax as you for deleting the whole range and it didn't work. I tried different permutations for quite a while and couldn't get it to work so I went back to what my original plan was. But I threw your `Range(ActiveCell, Range("C12:C9999").Find("!!End Measures!!").Offset(-1, 0)).EntireRow.Delete"` in and it did exactly what I wanted it to do. Thank you!! – Ryan Shinnick Nov 16 '17 at 21:52

2 Answers2

3

If you are just trying to delete all the rows from the ActiveCell down to the row containing "!!End Measures!!", you can replace your existing code with the one line:

Range(ActiveCell, Range("C12:C9999").Find("!!End Measures!!").Offset(-1, 0)).EntireRow.Delete

A slightly more robust version (which isn't dependent on ActiveCell, but simply deletes all rows between the "!!End Measures!!" and the previous non-blank cell) would be:

Dim EndMeasures As Range
Dim LastMeasure As Range
Set EndMeasures = Columns(3).Find("!!End Measures!!")
'Check that there are blank rows before deleting
If IsEmpty(EndMeasures.Offset(-1).Value) Then
    Set LastMeasure = EndMeasures.End(xlUp)
    Range(LastMeasure.Offset(1), EndMeasures.Offset(-1)).EntireRow.Delete
End If
YowE3K
  • 23,852
  • 7
  • 26
  • 40
  • 1
    I wish I could mark two answers correct. Yours does exactly what I wanted to do in a single line, but PGCodeRider was more answering my question directly. Thank you very much for your help! I really appreciate it. – Ryan Shinnick Nov 16 '17 at 22:00
  • Yow doesn't need the accepted answer as much as ME!! :) Yow, I gave you a tick because I'm a good person, yours is helpful, and I think I might earn a badge for it... hopefully! – pgSystemTester Nov 16 '17 at 22:18
  • @PGCodeRider Yes, upvoting someone else's answer helps you get the "sportsmanship" badge. I hate to say it, but I'm only 2/3rds of the way to getting it. I'm not sure whether that is because my answers are always the only answers worthy of upvotes ( ;) ), or whether it is because it only counts if your own answer also received at least one vote and getting votes in the VBA tag is always a hard task (too many newbies asking questions who don't have enough rep to actually vote for the answers). – YowE3K Nov 16 '17 at 22:26
  • @YowE3K You pretty much described me! Sorry buddy. I did upvote, but, as you said, I don't have enough rep..... – Ryan Shinnick Nov 17 '17 at 13:13
2

A couple things catch my attention.

One. It would be good to define what you're looping through.

Two. You're using Activecell in your loop, but I don't think that's what you want to be doing, I think you want SingleCell

Three. Your range is probably getting changed when you delete the row. Since you want no members, it would probably just be easier to restart the loop unless this is taking minutes to run. Consider this:

Dim ListofCells As Range, SingleCell as Range
StartItUp:
Set ListofCells = Range(ActiveCell, Range("C12:C9999").Find("!!End Measures!!"))

For Each SingleCell In ListofCells.Cells

   If SingleCell.Value = "!!End Measures!!" Then
        SingleCell.Offset(1, 0).Select
        Exit For
    ElseIf SingleCell.Value = "" Then
        SingleCell.EntireRow.Delete
        GoTo StartItUp
    End If
Next SingleCell
pgSystemTester
  • 8,979
  • 2
  • 23
  • 49
  • 1
    You were right, I didn't want to be using ActiveCell. I threw your code in, though, and it worked! Thank you so much. I will have to remember the "GoTo" function. – Ryan Shinnick Nov 16 '17 at 21:44
  • 1
    @RyanShinnick protip: you pretty much NEVER want to use `ActiveCell`, or `ActiveSheet`, or `ActiveWorkbook` - explicitly *or* implicitly. It's not because macro-recorder code is littered with implicit `ActiveSheet` references and `.Select` and `.Activate` calls, that *actual VBA code* needs to, or *should* do that. [Obligatory link](https://stackoverflow.com/q/10714251/1188513). – Mathieu Guindon Nov 16 '17 at 21:53
  • 1
    You're right @Mat'sMug. I just started with Excel VBA less than 2 weeks ago and am slowly getting the hang of it. As I keep making progress on the code that I'm writing I've slowly started phasing out my "ActiveCells" and "ActiveSheets". I will give your link a read (and probably bookmark it!) and try to incorporate that into my future code. Thank you!! – Ryan Shinnick Nov 16 '17 at 22:00