What's wrong with this code?
 

Hi!

I'm kinda new to VBA programming, so maybe you can help me...

When I run the following macro, the program seems to crash (doesn't
respond). Is there a fault in the code or is there another explanation? All
advice welcome!

Sub FindActuals()



Actualmonth = Worksheets("COOMonthWCYTD").Range("J5")

'On Error GoTo codeBreak
'For i = 5 To 10
For LookupRow = 3 To Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
Set LookupAcc = Sheets(10).Cells(LookupRow, 2)
For rwIndex = 3 To 505
With Sheets(Actualmonth).Cells(rwIndex, 2)
Sheets(Actualmonth).Select
Sheets(Actualmonth).Cells(rwIndex, 2).Activate
If LookupAcc.Value = ActiveCell.Value Then
OffsetCount = -1
If Not ActiveCell.Offset(OffsetCount, -1) =
ActiveCell.Offset _(0, -1) - 1 Then

If LookupAcc.Offset(0, -1).Value =
ActiveCell.Offset _(OffsetCount, 0).Value Then
LookupAcc.Offset(0, 5).Value =
ActiveCell.Offset _(OffsetCount, 2).Value

Else: OffsetCount = OffsetCount - 1

End If
End If
End If
End With
Next rwIndex
Next LookupRow
'Next i
'codeBreak: Exit Sub
End Sub

What's wrong with this code?
 

The first thing I notice is 3 nested loops: i=5 to 10, LookupRow=3 to ? (row
number), and rwIndex = 3 to 505. For each value of i, Lookup row has to run
for each row (let's say you have 500 rows??? - that would give 2500 passes
through your code), and then you do rwIndex another 500 times (approx) for
1,250,000 passes through the inside loop. That is a lot of work and would
take a long time to execute - it would appear "hung" until done, possibly
several minutes. You could check its progress periodically by Ctrl-Break and
use the immediate pane to print i, LookupRow, and rwIndex at that point, then
restart the code and do it again later.

If you could explain what you are trying to do there may be a way to avoid
some of the looping... the best way to speed up your code.
--
- K Dales


"PhilipsBernard" wrote:

Hi!

I'm kinda new to VBA programming, so maybe you can help me...

When I run the following macro, the program seems to crash (doesn't
respond). Is there a fault in the code or is there another explanation? All
advice welcome!

Sub FindActuals()



Actualmonth = Worksheets("COOMonthWCYTD").Range("J5")

'On Error GoTo codeBreak
'For i = 5 To 10
For LookupRow = 3 To Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
Set LookupAcc = Sheets(10).Cells(LookupRow, 2)
For rwIndex = 3 To 505
With Sheets(Actualmonth).Cells(rwIndex, 2)
Sheets(Actualmonth).Select
Sheets(Actualmonth).Cells(rwIndex, 2).Activate
If LookupAcc.Value = ActiveCell.Value Then
OffsetCount = -1
If Not ActiveCell.Offset(OffsetCount, -1) =
ActiveCell.Offset _(0, -1) - 1 Then

If LookupAcc.Offset(0, -1).Value =
ActiveCell.Offset _(OffsetCount, 0).Value Then
LookupAcc.Offset(0, 5).Value =
ActiveCell.Offset _(OffsetCount, 2).Value

Else: OffsetCount = OffsetCount - 1

End If
End If
End If
End With
Next rwIndex
Next LookupRow
'Next i
'codeBreak: Exit Sub
End Sub

What's wrong with this code?
 

Thanks for the reply!
What I am trying to do is to look up a pair of cells (with different layout)
inside a range of cells by first matching one cell, then the following cell
and if it's a double match assign a value to a cell next to the two original
cells. Does that make sense?
I now found out I missed a ' Do Loop' sequence by trying to eliminate some
lines and simplifying it. It does appear to take a very long time to process
the macro though...
So, if you have a suggestion for improving the speed of processing, it's
extremely welcome!



"K Dales" wrote:

The first thing I notice is 3 nested loops: i=5 to 10, LookupRow=3 to ? (row
number), and rwIndex = 3 to 505. For each value of i, Lookup row has to run
for each row (let's say you have 500 rows??? - that would give 2500 passes
through your code), and then you do rwIndex another 500 times (approx) for
1,250,000 passes through the inside loop. That is a lot of work and would
take a long time to execute - it would appear "hung" until done, possibly
several minutes. You could check its progress periodically by Ctrl-Break and
use the immediate pane to print i, LookupRow, and rwIndex at that point, then
restart the code and do it again later.

If you could explain what you are trying to do there may be a way to avoid
some of the looping... the best way to speed up your code.
--
- K Dales


"PhilipsBernard" wrote:

Hi!

I'm kinda new to VBA programming, so maybe you can help me...

When I run the following macro, the program seems to crash (doesn't
respond). Is there a fault in the code or is there another explanation? All
advice welcome!

Sub FindActuals()



Actualmonth = Worksheets("COOMonthWCYTD").Range("J5")

'On Error GoTo codeBreak
'For i = 5 To 10
For LookupRow = 3 To Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
Set LookupAcc = Sheets(10).Cells(LookupRow, 2)
For rwIndex = 3 To 505
With Sheets(Actualmonth).Cells(rwIndex, 2)
Sheets(Actualmonth).Select
Sheets(Actualmonth).Cells(rwIndex, 2).Activate
If LookupAcc.Value = ActiveCell.Value Then
OffsetCount = -1
If Not ActiveCell.Offset(OffsetCount, -1) =
ActiveCell.Offset _(0, -1) - 1 Then

If LookupAcc.Offset(0, -1).Value =
ActiveCell.Offset _(OffsetCount, 0).Value Then
LookupAcc.Offset(0, 5).Value =
ActiveCell.Offset _(OffsetCount, 2).Value

Else: OffsetCount = OffsetCount - 1

End If
End If
End If
End With
Next rwIndex
Next LookupRow
'Next i
'codeBreak: Exit Sub
End Sub

What's wrong with this code?
 

Afraid I don't have time to work out the code entirely, but here are a few
things:
1) Why the For i = 5 to 10? i is never used in the code, so this only has
the effect of making the code run 5 times over. Perhaps there is a reason
but it appears redundent.
2) Take out the Sheets().Select and Cells().Activate statements. They are
unnecessary since your code can find the cell values without the cursor
needing to be positioned there. It will also cause a lot of screen redraws
which will likely flicker (have you noted?) and takes valuable time. In
fact, set Application.ScreenUpdating = False at the top of your code and then
(IMPORTANT!) set it True again at the end to prevent any screen redraws that
can take time.
3) Instead of the loops, look at using the Range().Find and .FindNext
methods, e.g.:
Set FoundCell = Sheets(ActualMonth).Range("B:B").Find(LookupAcc.Va lue)
(Note: find can work either upwards - xlFindNext - or downwards -
xlFindPrevious. If not found, FoundCell will be Nothing. Check help from
Excel VBA for info on Find Method). As a built-in method Find will be more
efficient than looping.

So the basic outline:
1) Turn off screen updating
2) Eliminate the For i = 5 to 10 loop if you can
3) Use Find instead of loops to find the values you want
4) Find will return the cell where the value was found - use .Row and
..Column to find the row and column numbers, if needed, or just use the
..Offset method to read/write into surrounding cells.
5) Turn screenupdating back on
--
- K Dales


"PhilipsBernard" wrote:

Hi!

I'm kinda new to VBA programming, so maybe you can help me...

When I run the following macro, the program seems to crash (doesn't
respond). Is there a fault in the code or is there another explanation? All
advice welcome!

Sub FindActuals()



Actualmonth = Worksheets("COOMonthWCYTD").Range("J5")

'On Error GoTo codeBreak
'For i = 5 To 10
For LookupRow = 3 To Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
Set LookupAcc = Sheets(10).Cells(LookupRow, 2)
For rwIndex = 3 To 505
With Sheets(Actualmonth).Cells(rwIndex, 2)
Sheets(Actualmonth).Select
Sheets(Actualmonth).Cells(rwIndex, 2).Activate
If LookupAcc.Value = ActiveCell.Value Then
OffsetCount = -1
If Not ActiveCell.Offset(OffsetCount, -1) =
ActiveCell.Offset _(0, -1) - 1 Then

If LookupAcc.Offset(0, -1).Value =
ActiveCell.Offset _(OffsetCount, 0).Value Then
LookupAcc.Offset(0, 5).Value =
ActiveCell.Offset _(OffsetCount, 2).Value

Else: OffsetCount = OffsetCount - 1

End If
End If
End If
End With
Next rwIndex
Next LookupRow
'Next i
'codeBreak: Exit Sub
End Sub

What's wrong with this code?
 

Thanks a lot, this is a great help!
The i was something I included to run the macro over multiple sheets, but
since it took such a long time to process one sheet, I took the i out of the
code to speed up processing temporarily. Your comments help me to grasp the
hows and whys of VBA, many thanks again!


"K Dales" wrote:

Afraid I don't have time to work out the code entirely, but here are a few
things:
1) Why the For i = 5 to 10? i is never used in the code, so this only has
the effect of making the code run 5 times over. Perhaps there is a reason
but it appears redundent.
2) Take out the Sheets().Select and Cells().Activate statements. They are
unnecessary since your code can find the cell values without the cursor
needing to be positioned there. It will also cause a lot of screen redraws
which will likely flicker (have you noted?) and takes valuable time. In
fact, set Application.ScreenUpdating = False at the top of your code and then
(IMPORTANT!) set it True again at the end to prevent any screen redraws that
can take time.
3) Instead of the loops, look at using the Range().Find and .FindNext
methods, e.g.:
Set FoundCell = Sheets(ActualMonth).Range("B:B").Find(LookupAcc.Va lue)
(Note: find can work either upwards - xlFindNext - or downwards -
xlFindPrevious. If not found, FoundCell will be Nothing. Check help from
Excel VBA for info on Find Method). As a built-in method Find will be more
efficient than looping.

So the basic outline:
1) Turn off screen updating
2) Eliminate the For i = 5 to 10 loop if you can
3) Use Find instead of loops to find the values you want
4) Find will return the cell where the value was found - use .Row and
.Column to find the row and column numbers, if needed, or just use the
.Offset method to read/write into surrounding cells.
5) Turn screenupdating back on
--
- K Dales


"PhilipsBernard" wrote:

Hi!

I'm kinda new to VBA programming, so maybe you can help me...

When I run the following macro, the program seems to crash (doesn't
respond). Is there a fault in the code or is there another explanation? All
advice welcome!

Sub FindActuals()



Actualmonth = Worksheets("COOMonthWCYTD").Range("J5")

'On Error GoTo codeBreak
'For i = 5 To 10
For LookupRow = 3 To Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
Set LookupAcc = Sheets(10).Cells(LookupRow, 2)
For rwIndex = 3 To 505
With Sheets(Actualmonth).Cells(rwIndex, 2)
Sheets(Actualmonth).Select
Sheets(Actualmonth).Cells(rwIndex, 2).Activate
If LookupAcc.Value = ActiveCell.Value Then
OffsetCount = -1
If Not ActiveCell.Offset(OffsetCount, -1) =
ActiveCell.Offset _(0, -1) - 1 Then

If LookupAcc.Offset(0, -1).Value =
ActiveCell.Offset _(OffsetCount, 0).Value Then
LookupAcc.Offset(0, 5).Value =
ActiveCell.Offset _(OffsetCount, 2).Value

Else: OffsetCount = OffsetCount - 1

End If
End If
End If
End With
Next rwIndex
Next LookupRow
'Next i
'codeBreak: Exit Sub
End Sub