Solved: Loops sending me loopy

[philfer]

I have a big loop which scrolls through a lot of data concatenating, formatting, adding formulas etc.

i.e.

For i = 1 to lastrow

ActiveSheet.Range(Cells(i,2)).Value = etc etc

ActiveSheet.Range(Cells(i,5)).FormulaR1C1 = etc

If Right(ActiveSheet.Range(Cells(i,7)).Value,2)="XY" Then
ActiveSheet.Range(Cells(i,7)).Select
ActiveCell.EntireRow.Delete
i=i-1
lastrow = lastrow-1
Next i
End If

If ActiveSheet.Range(Cells(i,11)).Value = "FINAL" Then
ActiveSheet.Range(Cells(i,11)).Select
ActiveCell.EntireRow.Delete
i=i-1
lastrow = lastrow-1
Next i
End If

ActiveSheet.Range(Cells(i,15)).Value = etc etc

Next i


When I do the above it doesnt like the "Next i" lines that are in bold

Is there any way for me to jump to the top of the loop after I delete a row. Basically I have to go back to the top so that it looks at the earlier cells (i.e. (i,2) and (i,5)) once it has deleted the row and adjusted the counters

[Simon Lloyd]

As far as i can see you havent set what lastrow is you also set i to zero at the end of each statement it should look something like:

Dim i As Integer
Dim lastrow As Long
lastrow = Range("A65536").End(xlUp).Row
For i = lastrow To 1 Step -1

ActiveSheet.Range(Cells(i,2)).Value = etc etc

ActiveSheet.Range(Cells(i, 5)).FormulaR1C1 = etc

If Right(ActiveSheet.Range(Cells(i, 7)).Value, 2) = "XY" Then
ActiveSheet.Range(Cells(i, 7)).Select
ActiveCell.EntireRow.Delete

End If
Next i
For i = lastrow To 1 Step -1
If ActiveSheet.Range(Cells(i, 11)).Value = "FINAL" Then
ActiveSheet.Range(Cells(i, 11)).Select
ActiveCell.EntireRow.Delete
lastrow = lastrow - 1
End If
Next i

i havent tested it yet but its somewhere close!

[philfer]

Thanks for that Simon but its the additional Next i lines that are the problem.

Each time I try to F5 the code it says "Compile Error Next without For"

[Simon Lloyd]

my revised code above (rather than the one e-mailed to you as notification) should cure that problem, let me know if not!

[rbrhodes]

Hi Philfer,

If your code is working for you except the lack of the For/Loop pair, perhaps use a GoTo and a label:



For i = 1 to lastrow

ActiveSheet.Range(Cells(i,2)).Value = etc etc

ActiveSheet.Range(Cells(i,5)).FormulaR1C1 = etc

If Right(ActiveSheet.Range(Cells(i,7)).Value,2)="XY" Then
ActiveSheet.Range(Cells(i,7)).Select
ActiveCell.EntireRow.Delete
i=i-1
lastrow = lastrow-1

goto nextI

End If

If ActiveSheet.Range(Cells(i,11)).Value = "FINAL" Then
ActiveSheet.Range(Cells(i,11)).Select
ActiveCell.EntireRow.Delete
i=i-1
lastrow = lastrow-1

goto nextI

End If

ActiveSheet.Range(Cells(i,15)).Value = etc etc

nextI:

Next i

[Paul_Hossler]

The Next i is only used as a loop block terminator. You can't use it inside a loop block as a way to exit the current interation and go get the next i.

There is an 'Exit For' statement but that will go to the statment after the Next i, i.e. stopping the whole loop

Either use a If/Endif construct or the Goto suggested by rbrhosds.

Paul

[rory]

Goto is the devil's work and should be avoided unless you really like spaghetti code. Try this:
Dim i As Long
Dim lastrow As Long
lastrow = Range("A65536").End(xlUp).Row
For i = lastrow To 1 Step -1
If Right(ActiveSheet.Cells(i, 7).Value, 2) = "XY" Or _
ActiveSheet.Cells(i, 11).Value = "FINAL" Then

ActiveSheet.Rows(i).Delete
Else
' I assumed this would not affect the above conditions;
' if it does you should move it back above the If...End If block!
' if it doesn't then there's no point doing this if you are going to delete the row

ActiveSheet.Cells(i, 2).Value = etc
ActiveSheet.Cells(i, 5).FormulaR1C1 = etc
End If
Next i

[Bob Phillips]

Goto is the devil's work and should be avoided unless you really like spaghetti code.

Comments like this really are nonsense, and do not add to helping people develop their coding skills one iota.

Just look at the OP's code, he used Goto, and is his code spaghetti code? No, of course it is not, it is perfectly readable and understandable.

If used sparingly and in a structured manner, Gotos are no more of a coding anathema than Loops or Ifs. True, they interrupt the control flow, but so does Exit For, Exit Sub. Are you suggesting that they too are devils work?

Gotos are bad if they are over-used, if they are used in a non-structured manner, or if the code is badly designed and/or badly written. But guess what, you don't need Gotos to create poor code in exactly the same way. Spaghetti coders create spaghetti code, not Gotos.

I do not advocate using Gotos, and you have showed how it can be done without, but I am challenging unqualified statements. The construct of VB means that Goto's are unavoidable, how would you create proper error handling without Gotos?

[rbrhodes]

Thanks!

I don't use Goto's much either (my address is 667 - I'm the neighbour of the beast...<g>) but it seemed llike a simple proposal to avoid the "next I" construct that the OP had presented.

Cheers!

[rory]

You are correct - it was a flippant comment that I was about to edit before our internet connection went down until just now. Goto's are indeed necessary for proper error handling, but I happen to think they should be avoided otherwise. (I fail to see where the OP used a single Goto, but that's by the by) As with all rules, there are exceptions, but I think it's a good rule of thumb since I don't think I've ever seen the need for a goto that wasn't related to error handling. I also happen to think that 'Exit For' et al (I'll ignore Exit Sub since that pretty much terminates any spaghetti) are considerably clearer than a goto.
I will try and restrain my need for flippancy in future as you are quite right that it doesn't help people to learn.

[Bob Phillips]

You are correct - it was a flippant comment that I was about to edit before our internet connection went down until just now. Goto's are indeed necessary for proper error handling, but I happen to think they should be avoided otherwise.

I tend not to use them, but that is a style preference, it is nothing to do with the devil or spaghetti code.


(I fail to see where the OP used a single Goto, but that's by the by)

By making that point I can ask you why you said what you said, but we both know that we are both referring to the post that had the code with Goto's, so what the ...


I also happen to think that 'Exit For' et al (I'll ignore Exit Sub since that pretty much terminates any spaghetti) are considerably clearer than a goto.

Then pray tell me why because I would argue exactly the oppoosite on clarity grounds. Both break the flow, but at least a Goto goes to a label, so it is blindingly obvious, whereas in a set of say 3 For ... Next, it takes time to ascertain which, especially if the code is of the Italian variety.

[rory]

Exit For just exits the loop it's in so you know you are leaving a For...Next loop and that you resume after the Next statement. A goto could go to a label anywhere in the code and is unclear as to what exactly you are leaving. I also don't like having unnecessary labels in the code but, as I said, it's an opinion.
As for spaghetti code, all the worst instances I have seen have involved people using Goto statements to jump back and forth in and out of loops. They seem to be used as an easy fix for a problem that usually ought to cause a rewrite of code, therefore I think it's a good practice to avoid them other than in error handling.

[unmarkedhelicopter]

I agree that goto's should be avoided, but ...
I have written code that does extensive checking on a set of conditions, where different analysis is required for different conditions, looking for a change and then repeating the loop, if a change does not occur then do another 'outer' loop etc. these can be nested such that (in one case) 8 different logical analyses are nested, then comes a 'guess' stage. Any of the 8 loops can find an error in this guess and so I wish to remove the guess, dependant (and thus incorrect) analysis and continue with the next 'guess'. This has been honed to the application and operates at least 10 times faster than any other (vba) code I've seen to do the same and guess what !!! it uses a goto. The only error checking in the whole code (apart from data input) is to spot where a guess leads to a contradiction so is not 'an error condition' per sae. You may say that this proves that goto's are valid, and indeed, inclusion in vba syntax should also say this but truth is there are many such crutches in vba and people should be pointed away from dependance on many of them.
XLD, ASSiduously avoids the use of evalute and equates that to a plaything of beelsibub (sp ?) but I notice he has used it in many solutions is the past. Far be it from me to highlight this hypocrasy, or maybe it's just a double standard ???

[Bob Phillips]

Exit For just exits the loop it's in so you know you are leaving a For...Next loop and that you resume after the Next statement. A goto could go to a label anywhere in the code and is unclear as to what exactly you are leaving. I also don't like having unnecessary labels in the code but, as I said, it's an opinion.
As for spaghetti code, all the worst instances I have seen have involved people using Goto statements to jump back and forth in and out of loops. They seem to be used as an easy fix for a problem that usually ought to cause a rewrite of code, therefore I think it's a good practice to avoid them other than in error handling.

Well I don't want to argue with you because I don't think we disagree on the premise, just on the detail, and how it is stated.

My point really was that I don't believe that Goto's are bad, even though I choose to avoid them. Spaghetti code is spaghetti code. I don't believe that you write spaghetti code, and that it would be not become spaghetti even if you added Gotos. IMO, if Gotos always go forward, I don't see a problem. In my experience, when someone says Gotos are a bad programming practice, they are usually fresh out of college, they read it somewhere in a book, and they cannot back it up with anything substantial other than reciting the mantra.

If you want spaghetti code, I used to code in Cobol many years ago. Cobol allowed the use of alterable Gotos. I forget the exact syntax, but you would have a line something like



Label1: Goto Label_nofind


and then somewhere else in the code you could have



Alter Label1 To Goto Label_NowFound


Imagine the chaos you can create with that. I was working for a firm that bought an application that had been written by a bunch of programmers striaght out of their first Cobol course, and they had used these ALterable Gotos everywhere. We weer migrating to a machine where this ridiculous syntax was invalid, so we had to upgrade the application, and it was a nightmare. Luckily I had one guy who was absolutely great at it, he could follow it through and kep good notes and get a good handle on its logic. Without him we would have been dead.

After that, I have never seen the hysteria over Gotos.

[Bob Phillips]

XLD, ASSiduously avoids the use of evalute and equates that to a plaything of beelsibub (sp ?) but I notice he has used it in many solutions is the past. Far be it from me to highlight this hypocrasy, or maybe it's just a double standard ???

Spelling? Beelzebub. Hypocrisy.

No, you are just wrong, but I love your subtle use of the capitals.

I assiduously avoid the shortcut syntax that uses evaluating, that is [A1].

If you know how you can resolve an array formula in VBA or SUMPRODUCT in VBA without evaluating, then pray tell me and I can avoid that too. But I won't hold my breath.

[unmarkedhelicopter]

Ah !
Subtlety was never my
Strong point :) catching on yet ??? ...

Now how did I know you'd be spot on when it came to spelling Beelzebub and Hypocrisy ? :) catching on yet ??? ...

[rory]

Yes, I think we're both coming from the same place basically. It's true that Goto is not inherently bad, but the way I usually see it used is. I've never used COBOL (and now I'm extremely glad about that) so I guess it's all relative - immutable Gotos can cause enough havoc without them being able to metamorphose halfway through a routine. Even Kafka would shudder...

[Bob Phillips]

It's true that Goto is not inherently bad, but the way I usually see it used is.

And that is probably the problem with them.

And you could probably say the same about Selects. But that is another story ...

[rory]

Oddly enough, that was exactly the thought that occurred to me this morning - occasionally necessary, but otherwise I try to avoid them.

[rbrhodes]

We seem to have lost the OP...

BTW: Spaghettie? Beezledude? Hypocryte? Speeling?


For i = 1 To lastrow
If <something> Then
'Do stuff
Goto next_i 'then do next i
ElseIf <something> Then
'Do sother stuff
Goto next_i 'then do next i
End If

Next_i:

Next i



The label seems clear to me <?> Funny stuff none the less!

Thanks for the smile of the day!

[rbrhodes]

For i = 1 To lastrow
If <something> Then
'Do stuff
'then do next i
Goto nextI
ElseIf <something> Then

Goto nextI
End If

NextI:

Next i

[rbrhodes]

For i = 1 To lastrow

If something Then

'Do stuff
'then do next i
Goto nextI

ElseIf <something> Then

'Do other stuff
'then do next i
Goto nextI
End If

NextI:

Next i

[rbrhodes]

We


For i = 1 To lastrow

If something = true Then

'Do stuff
'then do next i
Goto nextI

ElseIf somethingelse = true Then

'Do other stuff
'then do next i
Goto nextI
End If

NextI:

Next i

[rbrhodes]

We seem to have lost the OP.

BTW: Spaghetti? Beezledude?


For i = 1 To lastrow

If something = true Then

'Do stuff
'then do next i
Goto nextI

ElseIf somethingelse = true Then

'Do other stuff
'then do next i
Goto nextI
End If

NextI:

Next i

[rory]

Very nice example of a pointless Goto! :)