Could this be a loop?

  • Thread starter Thread starter Guest
  • Start date Start date
G

Guest

Any help would be appreciated. I have been told that GoTo used in anything
other than error handling should be avoided. THe code below works just like
I want it to, but I was wondering if anyone had any ideas for removing the
GoTo. I have tried a
Do Until, but that didn't give me the same results.

TIA,

Kim

For i = 1 To lIndex

TryAgain:
If MyRV(i).sMonName = .ActiveCell.Offset(iPlace, 1).Value Then
.ActiveCell.Offset(iPlace, 0).Value = MyRV(i).sEngineFamily
.ActiveCell.Offset(iPlace, 2).Value = MyRV(i).dRepairCount
iPlace = iPlace + 1
Else
iPlace = iPlace + 1
GoTo TryAgain
End If
Next i
 
Kim said:
Any help would be appreciated. I have been told that GoTo used in anything
other than error handling should be avoided. THe code below works just like
I want it to, but I was wondering if anyone had any ideas for removing the
GoTo. I have tried a
Do Until, but that didn't give me the same results.

TIA,

Kim

For i = 1 To lIndex

TryAgain:
If MyRV(i).sMonName = .ActiveCell.Offset(iPlace, 1).Value Then
.ActiveCell.Offset(iPlace, 0).Value = MyRV(i).sEngineFamily
.ActiveCell.Offset(iPlace, 2).Value = MyRV(i).dRepairCount
iPlace = iPlace + 1
Else
iPlace = iPlace + 1
GoTo TryAgain
End If
Next i
Click to expand...

You can leave it out and simplify:

For i = 1 To lIndex
If MyRV(i).sMonName = .ActiveCell.Offset(iPlace, 1).Value Then
.ActiveCell.Offset(iPlace, 0).Value = MyRV(i).sEngineFamily
.ActiveCell.Offset(iPlace, 2).Value = MyRV(i).dRepairCount
End If
iPlace = iPlace + 1
Next i

HTH
Matthias Kläy
 
Kim said:
Any help would be appreciated. I have been told that GoTo used in
anything
other than error handling should be avoided. THe code below works just
like
I want it to, but I was wondering if anyone had any ideas for removing the
GoTo. I have tried a
Do Until, but that didn't give me the same results.
Click to expand...

Try this:

For i = 1 To lIndex
Do Until MyRV(i).sMonName <> .ActiveCell.Offset(iPlace, 1).Value
.ActiveCell.Offset(iPlace, 0).Value = MyRV(i).sEngineFamily
.ActiveCell.Offset(iPlace, 2).Value = MyRV(i).dRepairCount
iPlace = iPlace + 1
Loop
iPlace = iPlace + 1
Next i

Tom Lake
 
First of all, this looks like an Excel problem, not an Access one, but
still...

Second of all, if you want someone else to read your code then you really
need to comment it. You are going to be trying to read this in three
months' time and it'll look like greek!! Approx 40% comment lines is a
good average to aim at for fairly simple code.

Third: yes, you don't need the goto at all.

For i = 1 To lIndex

TryAgain:
If MyRV(i).sMonName = .ActiveCell.Offset(iPlace, 1).Value
Then
.ActiveCell.Offset(iPlace, 0).Value =
MyRV(i).sEngineFamily .ActiveCell.Offset(iPlace,
2).Value = MyRV(i).dRepairCount iPlace = iPlace + 1
Else
iPlace = iPlace + 1
GoTo TryAgain
End If
Next i
Click to expand...

' loop through the array
For i = 1 To lIndex

' look through rows until we find a match
Do While True
' compare first column with MonName
If MyRV(i).sMonName = .ActiveCell.Offset(iPlace, 1).Value
Then
' it's there, so copy the values across
.ActiveCell.Offset(iPlace, 0).Value = MyRV(i).sEngineFamily
.ActiveCell.Offset(iPlace, 2).Value = MyRV(i).dRepairCount

' point at next row for the next member of the array
iPlace = iPlace + 1

' hop out and get the next member
Exit Do

Else
' not found - try the next row
iPlace = iPlace + 1

End If
Loop
Next i



This approach also reveals the big danger of this bit of code. There is
no check to see when iPlace takes the comparison off the end of the
worksheeet. You need to change the inner loop control line to

Do While Not IsEmpty(.ActiveCell.Offset(iPlace,1))


and then probably put a similar control in the For...Next loop to jump
right out when there are no more rows.

Just a thought


Tim F
 
Thanks for the help Tim. Your approach worked. You are right about Excel,
but this procedure is called in Access.
 
This is a better way. The code below does what you are doing, but is is
better programming practice. It appears your code was incrementing iPlace to
find the match then entering values in the cells you are looking for in the
inner loop. The outer loop is doing the inner from 1 through lIndex. The
one thing I am not sure about is iPlace. It never gets reset so that
assuming when i = 1, you start iPlace from 1 and it goes to 4 before you find
a match, then when you do the next iteration of i (i=2), iPlace will start at
4. This is suspicious and unusual. Normally, you would set the starting
value for iPlace right before the Do Until statement.

For i = 1 To lIndex
Do Until MyRV(i).sMonName = .ActiveCell.Offset(iPlace, 1).Value
iPlace = iPlace + 1
Loop
.ActiveCell.Offset(iPlace, 0).Value = MyRV(i).sEngineFamily
.ActiveCell.Offset(iPlace, 2).Value = MyRV(i).dRepairCount
Next i
 
You're already in a loop. The 'NEXT I' sends the code back to the
FOR...TO statement automatically until i = lIndex. I am assuming that
you are setting lIndex to a value before the statement.
 
Everybody seems to be missing the point on this one. There are two loops in
the code. Kim is just trying to understand nested loops. the inner loop is
incrementing oPlace until she gets this match:
MyRV(i).sMonName = .ActiveCell.Offset(iPlace, 1).Value

Then it rolls up to the outer loop incrementing i and doing the inner loop
again. Her only issue here is substituting the Do Until for the Goto. Now,
it is possible that she really only wants to get to a match one time, but
that is not what her code is doing.
 
Back
Top