I have seen it stated that it is undesirable, in a For...Next loop, e.g.,

For i = 1 to 10
'Do Something
Next

to change the index (i, in the above illustration) programatically.

1. Is this a commonly accepted taboo?

2. If so, why is it thought to be undesirable?

Thanks,
Alan Beban

The only danger that I can see is if the index gets set back, it could
create an infinite loop.

I have never experienced a problem just moving the index along its
step-path, even going beyond its loop limit.

It's like everything I guess, if you understand the implications, and know
the data, you can weigh up the risks.

--
---
HTH

Bob

(there's no email, no snail mail, but somewhere should be gmail in my addy)



"Alan Beban" wrote in message
...
I have seen it stated that it is undesirable, in a For...Next loop, e.g.,

For i = 1 to 10
'Do Something
Next

to change the index (i, in the above illustration) programatically.

1. Is this a commonly accepted taboo?

2. If so, why is it thought to be undesirable?

Thanks,
Alan Beban

Hi Alan.

I suppose that an iterative programming challenge might be OK for
programmatic index modification. But otherwise, the way I interpret this, is
that what is being changed must inherently be tightly coupled with what is
changing it. In the real world that could result in disparate code fragments
evaluating similar business logic differently, and might non-obviously have
to be maintained concurrently.

Tim

"Alan Beban" wrote in message
...
I have seen it stated that it is undesirable, in a For...Next loop, e.g.,

For i = 1 to 10
'Do Something
Next

to change the index (i, in the above illustration) programatically.

1. Is this a commonly accepted taboo?

2. If so, why is it thought to be undesirable?

Thanks,
Alan Beban

I personally avoid changing the index as I find it too difficult to debug
when things go wrong. The loops either run long (infinite loop possibly) or
end too soon and I end up patching things up in a bandaid kind of fashion.
Nothing written in stone here but I have never run into a situation that I
could not work around needing to change the index. The way I see it it is
kind of like Goto's... better to avoid them than to fix them.
--
HTH...

Jim Thomlinson


"Alan Beban" wrote:

I have seen it stated that it is undesirable, in a For...Next loop, e.g.,

For i = 1 to 10
'Do Something
Next

to change the index (i, in the above illustration) programatically.

1. Is this a commonly accepted taboo?

2. If so, why is it thought to be undesirable?

Thanks,
Alan Beban

Thanks to Bob Phillips, Tim Zych and Jim Thomlinson for taking the time
to respond (although I must say I wasn't able to follow Tim's response).
Just for the record, the situation that prompted my post was code to
delete all duplicate rows in a range. The following, in which RowsEqual
is a function that returns True or False depending on whether two rows
in a range or a two-dimensional array have all the same elements, was
what I coded:

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
Set rng = Range("DataRange").Rows
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then
rng(i).Delete
i = i - 1
j = i
End If
Next j
Next i
End Sub

Alan Beban

Jim Thomlinson wrote:
I personally avoid changing the index as I find it too difficult to debug
when things go wrong. The loops either run long (infinite loop possibly) or
end too soon and I end up patching things up in a bandaid kind of fashion.
Nothing written in stone here but I have never run into a situation that I
could not work around needing to change the index. The way I see it it is
kind of like Goto's... better to avoid them than to fix them.

Your rng.Rows.Count is going to change as you delete rows and that is not a
good idea. Not onny are you changing i but you are also changing the range in
which it is moving...

This is just my preference but when I am deleteing I always create a range
object of all of the cells that want to delete within the loop structure and
then do one big delete of all of the cells I found at the end. It is more
effiecient and it avoids problems with the range changing while I am looking
through it. Just my two cents... With your code it would look like this...

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
dim rngToDelete as range

Set rng = Range("DataRange")
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then
if rngToDelete is nothing then
Set rngtoDelete = rng(i)
else
Set rngToDelete = union(rngToDelete, rng(i))
end if
End If
Next j
Next i
if not rngToDelete is nothing then rngToDelete.Delete
End Sub
--
HTH...

Jim Thomlinson


"Alan Beban" wrote:

Thanks to Bob Phillips, Tim Zych and Jim Thomlinson for taking the time
to respond (although I must say I wasn't able to follow Tim's response).
Just for the record, the situation that prompted my post was code to
delete all duplicate rows in a range. The following, in which RowsEqual
is a function that returns True or False depending on whether two rows
in a range or a two-dimensional array have all the same elements, was
what I coded:

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
Set rng = Range("DataRange").Rows
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then
rng(i).Delete
i = i - 1
j = i
End If
Next j
Next i
End Sub

Alan Beban

Jim Thomlinson wrote:
I personally avoid changing the index as I find it too difficult to debug
when things go wrong. The loops either run long (infinite loop possibly) or
end too soon and I end up patching things up in a bandaid kind of fashion.
Nothing written in stone here but I have never run into a situation that I
could not work around needing to change the index. The way I see it it is
kind of like Goto's... better to avoid them than to fix them.

Hi Jim,

My data range contained the following:

1 2 3 4
a b c d
1 2 3 4
a b c d
5 6 7 8
1 2 3 4
a b c d
1 2 3 4
a b c d
5 6 7 8

When I run my code on it the result is

1 2 3 4
a b c d
5 6 7 8

When I first attempted to run your code, I got a compiler error, Type
mismatch, at the indicated line. I then changed your code from

Set rng = Range("DataRange") to

Set rng = Range("DataRange").Rows with this result:

1 2 3 4
a b c d


5 6 7 8

Your comment did make me rethink my code and I simplified it to

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
Set rng = Range("DataRange").Rows
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then
rng(i).Delete
End If
Next j
Next i
End Sub

Thanks for your interest. In case you wanted to check it yourself, here
is the RowsEqual function (watch for wordwrap):

Function RowsEqual(Row1, Row2)
'This function checks to see if two "rows"
'of an array or range are equal; it returns
'True if they are, False if they are not.

k = True

'Check to see that input rows are arrays or
'multicell ranges.
If (IsArray(Row1) And IsArray(Row2)) Then

'Convert input ranges to arrays.
arr1 = Row1: arr2 = Row2

'Loop to see if all elements are the same.
For i = LBound(arr1) To UBound(arr1)
For j = LBound(arr2) To UBound(arr2, 2)
If Not (arr1(i, j) = arr2(i, j)) Then RowsEqual =
False: Exit Function
Next
Next
RowsEqual = True

'If either input row is not an array or multicell
'range, give error message
Else
RowsEqual = "This function accepts only arrays and multicell
ranges"
End If

End Function


Jim Thomlinson wrote:
Your rng.Rows.Count is going to change as you delete rows and that is not a
good idea. Not onny are you changing i but you are also changing the range in
which it is moving...

This is just my preference but when I am deleteing I always create a range
object of all of the cells that want to delete within the loop structure and
then do one big delete of all of the cells I found at the end. It is more
effiecient and it avoids problems with the range changing while I am looking
through it. Just my two cents... With your code it would look like this...

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
dim rngToDelete as range

Set rng = Range("DataRange")
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then '<------Type mismatch
if rngToDelete is nothing then
Set rngtoDelete = rng(i)
else
Set rngToDelete = union(rngToDelete, rng(i))
end if
End If
Next j
Next i
if not rngToDelete is nothing then rngToDelete.Delete
End Sub

Alan,

Notwithstanding what I said earlier, I tend to agree with Jim in that I
would avoid setting an index from within an inner loop, the debugging could
get tricky! In other words, I don't like those risks :-).

Also, by saving up the deletes until the end as Jim does, you stop the
screen flashing as the screen gets re-painted when deleting.

I amended Jim's code and this worked for me

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
Dim rngToDelete As Range

Set rng = Range("DataRange").Rows
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then
If rngToDelete Is Nothing Then
Set rngToDelete = Rows(i)
Else
Set rngToDelete = Union(rngToDelete, Rows(i))
End If
End If
Next j
Next i
If Not rngToDelete Is Nothing Then rngToDelete.Delete
End Sub

--
---
HTH

Bob

(there's no email, no snail mail, but somewhere should be gmail in my addy)



"Alan Beban" wrote in message
...
Hi Jim,

My data range contained the following:

1 2 3 4
a b c d
1 2 3 4
a b c d
5 6 7 8
1 2 3 4
a b c d
1 2 3 4
a b c d
5 6 7 8

When I run my code on it the result is

1 2 3 4
a b c d
5 6 7 8

When I first attempted to run your code, I got a compiler error, Type
mismatch, at the indicated line. I then changed your code from

Set rng = Range("DataRange") to

Set rng = Range("DataRange").Rows with this result:

1 2 3 4
a b c d


5 6 7 8

Your comment did make me rethink my code and I simplified it to

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
Set rng = Range("DataRange").Rows
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then
rng(i).Delete
End If
Next j
Next i
End Sub

Thanks for your interest. In case you wanted to check it yourself, here is
the RowsEqual function (watch for wordwrap):

Function RowsEqual(Row1, Row2)
'This function checks to see if two "rows"
'of an array or range are equal; it returns
'True if they are, False if they are not.

k = True

'Check to see that input rows are arrays or
'multicell ranges.
If (IsArray(Row1) And IsArray(Row2)) Then

'Convert input ranges to arrays.
arr1 = Row1: arr2 = Row2

'Loop to see if all elements are the same.
For i = LBound(arr1) To UBound(arr1)
For j = LBound(arr2) To UBound(arr2, 2)
If Not (arr1(i, j) = arr2(i, j)) Then RowsEqual = False:
Exit Function
Next
Next
RowsEqual = True

'If either input row is not an array or multicell
'range, give error message
Else
RowsEqual = "This function accepts only arrays and multicell
ranges"
End If

End Function


Jim Thomlinson wrote:
Your rng.Rows.Count is going to change as you delete rows and that is not
a good idea. Not onny are you changing i but you are also changing the
range in which it is moving...

This is just my preference but when I am deleteing I always create a
range object of all of the cells that want to delete within the loop
structure and then do one big delete of all of the cells I found at the
end. It is more effiecient and it avoids problems with the range changing
while I am looking through it. Just my two cents... With your code it
would look like this...

Sub abtest4()
Dim rng As Range, i As Integer, j As Integer
dim rngToDelete as range

Set rng = Range("DataRange")
For i = rng.Rows.Count To 2 Step -1
For j = i - 1 To 1 Step -1
If RowsEqual(rng(i), rng(j)) Then '<------Type mismatch
if rngToDelete is nothing then
Set rngtoDelete = rng(i)
else
Set rngToDelete = union(rngToDelete, rng(i))
end if
End If
Next j
Next i
if not rngToDelete is nothing then rngToDelete.Delete
End Sub