VBA excel: For Each..Next doesn't go through the selected Range?

Question

I am trying to run my code:

Sub Test()
    Dim vuosi As Integer
    Dim kk As Integer
    Dim cF As Range
    Dim c As String
    Dim cell As Range
    vuosi = Application.InputBox(Prompt:="Syötä vuosi, jota haluat tarkastella.")
    kk = Application.InputBox(Prompt:="Syötä kuukausi(1-12), jota haluat tarkastella.")
    If vuosi = 2014 Then
        c = "BU"
    ElseIf vuosi = 2015 Then
        c = "CG"
    ElseIf vuosi = 2016 Then
        c = "CS"
    End If
    ActiveSheet.Range("F11:F60").Select
    For Each cell In Selection
        Cells(ActiveCell.Row, c).Activate  
        Set cF = Range(ActiveCell.Offset(0, kk - 12), ActiveCell.Offset(0, kk)).Find(What:=1, LookIn:=xlFormulas, LookAT:=xlPart, SearchOrder:=xlByColumns, MatchCase:=False, SearchFormat:=False)
        If Not cF Is Nothing Then
            Cells(ActiveCell.Row, "F").Interior.ColorIndex = 6
        End If
    Next cell
End Sub

It doesn't work properly. It seems the For Each loop goes through only the first line. Can anyone tell me why?

The program should go through all the cells in column F. For each row it checks if there is value 1 found in specific cells. If yes, the cell in F column should be painted yellow. Else the program continues till the last value found in column F. (in my Test I used just Range("F11:F60")

You never actually use the `Cell` variable. You should be using `Cells(Cell.Row, c).Activate ` — Rory, May 13 '15 at 08:02

score 1 · Accepted Answer · edited May 23 '17 at 12:00

I have some observations with regards to your code. Let me try and cover what all I noticed.

A) When working with Excel rows, avoid declaring them as Integers. Post Excel 2007, the rows have gone upto 1048576 which is too big a number for an Integer. You can get away when working with columns but may have problems with rows. It is a good habit to declare them as Long

B) You are assuming that the user will always enter values in the Input boxes. What if the user enters a blank or presses Cancel? Trap those instances. If you want only numbers to be entered in the Inputboxes then use Type:=1 with it. For more details read up on the Application.InputBox Method in the Excel Help.

C) Avoid the use of .Select/.Activate Your code can easily run with out selecting anything. You may want to see THIS

Is this what you are trying? (Untested)

Sub Test()
    Dim vuosi As Integer, kk As Long
    Dim rng As Range, aCell As Range, rngToFind As Range, cF As Range
    Dim c As String
    Dim ws As Worksheet

    vuosi = Application.InputBox(Prompt:="Syötä vuosi, jota haluat tarkastella.", Type:=1)

    If vuosi < 1 Then Exit Sub

    kk = Application.InputBox(Prompt:="Syötä kuukausi(1-12), jota haluat tarkastella.", Type:=1)

    If kk < 1 Then Exit Sub

    If vuosi = 2014 Then c = "BU"
    If vuosi = 2015 Then c = "CG"
    If vuosi = 2016 Then c = "CS"

    Set ws = ThisWorkbook.Sheets("Sheet1")

    With ws
        Set rng = ws.Range("F11:F60")

        For Each aCell In rng
            Set rngToFind = .Range(aCell.Offset(0, kk - 12), aCell.Offset(0, kk))

            Set cF = rngToFind.Find(What:=1, LookIn:=xlFormulas, LookAT:=xlPart, _
                                    SearchOrder:=xlByColumns, MatchCase:=False, _
                                    SearchFormat:=False)

            If Not cF Is Nothing Then .Range("F" & aCell.Row).Interior.ColorIndex = 6
        Next cell
    End With
End Sub

score 0 · Answer 2 · answered May 13 '15 at 08:02

This page will explain how ActiveCell.Row refers to only a single row in a selection.

In this case, you can replace:

ActiveSheet.Range("F11:F60").Select
    For Each cell In Selection
        Cells(ActiveCell.Row, c).Activate

with:

Set rng = ActiveSheet.Range("F11:F60")
    For Each cell In rng
        Cells(cell.Row, c).Activate

Also, rng should be declared as Range beforehand.

0m3r · Answer 3 · 2015-05-13T09:35:53.467

For nested If...End If statements, make sure there is a correctly matched If...End If structure inside each enclosing If...End If structure.

Sub Test()
    Dim Vuosi As Integer
    Dim KK As Integer
    Dim CF As Range
    Dim C As String
    Dim cell As Range

    Vuosi = Application.InputBox(Prompt:="Enter the year you wish to view.")
    KK = Application.InputBox(Prompt:="Enter the month (1-12) you wish to view.")

    If Vuosi = 2014 Then
        C = "BU"
    Else
    If Vuosi = 2015 Then
        C = "CG"
    Else
    If Vuosi = 2016 Then
        C = "CS"
    End If

    ActiveSheet.Range("F11:F60").Select
        For Each cell In Selection
            Cells(ActiveCell.Row, C).Activate
            Set CF = Range(ActiveCell.Offset(0, KK - 12), _
            ActiveCell.Offset(0, KK)).Find(What:=1, _
            LookIn:=xlFormulas, LookAT:=xlPart, _
            SearchOrder:=xlByColumns, MatchCase:=False, SearchFormat:=False)
            If Not CF Is Nothing Then
                Cells(ActiveCell.Row, "F").Interior.ColorIndex = 6
            End If
        Next cell
    End If
    End If
End Sub

score -2 · Answer 4 · answered May 13 '15 at 07:45

-2

You change your selection within your loop. Each time excel reach for each cell in selection it evaluates selection again, so it runs only once.
Instead of

...
ActiveSheet.Range("F11:F60").Select
For Each cell In Selection
...

try

...
dim r as range
... 
set r = activesheet.range("F11:F60")
For Each cell In Selection.cells
...

answered May 13 '15 at 07:45

Máté Juhász

2,197
1
19
40

That's not actually true. The `Selection` is not re-evaluated. – Rory May 13 '15 at 08:01

VBA excel: For Each..Next doesn't go through the selected Range?

4 Answers4