Looping & copying cells

Question

Generally my macro goes through every "O" cell, checks if the row meets given requirements (not mentioned in this part of code) and copies surrounding cells on the side. I have two columns used in this part: "contract no"(M), "date"(O). The problem is that I try to use below method to go up to last the contract number and copy it as well. I do not get any error but the contract cell value does not paste. Could you tell me what I've done wrong?

If ActiveCell.Offset(0, -2) = "" Then
    'Go up find contract number copy
    ActiveCell.Offset(0, -2).Select
    Do Until ActiveCell.Value <> ""
        ActiveCell.Offset(-1, 0).Select                                                                     
    Loop                                                     
    ActiveSheet.Range("M" & ActiveCell.Row).Copy _
        Destination:=ActiveSheet.Range("V" & ActiveCell.Row)
    'Go down and return to the last active cell
    Do Until ActiveCell.Value <> ""
        ActiveCell.Offset(1, 0).Select 
    Loop
    ActiveCell.Offset(0, 2).Select
End If

`.Range("V" & ActiveCell.Row)` is wrong syntax. Try `ActiveSheet.Cells(ActiveCell.Row, "V")` — Variatus, Feb 14 '18 at 10:06
@Variatus - what's wrong with `Range("V" & ActiveCell.Row)`? — SJR, Feb 14 '18 at 10:14
You should read [this](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba). — SJR, Feb 14 '18 at 10:16
@Variatus Thank you I'll keep it in mind. However it didn't help me in this case. — Tomek, Feb 14 '18 at 10:24
My mistake - end of day benightedness! I will try to make up. — Variatus, Feb 14 '18 at 10:25

score 1 · Answer 1 · edited Jun 20 '20 at 09:12

You didn't select the desired cell

Problem lies in this loop:

'Selecting cell from a column to the left
ActiveCell.Offset(0, -2).Select
'Condition: cell value is not empty string
Do Until ActiveCell.Value <> ""
    'Selecting cell from previous row in the same column
    ActiveCell.Offset(-1, 0).Select
Loop

You're leaving the loop before you can .Select a cell.

Use this loop instead:

'Selecting cell from a column to the left
ActiveCell.Offset(0, -2).Select
'Condition: cell value is not empty string
Do
    'Selecting cell from previous row in the same column
    ActiveCell.Offset(-1, 0).Select
Loop Until ActiveCell.Value <> ""

score 0 · Answer 2 · answered Feb 14 '18 at 10:29

Try not to use ActiveCell Your code can do quite unpredictable things to your worksheet if the wrong cell was selected, and so can my "improvement" thereof below.

Sub FindAndCopy()

    Dim Ws As Worksheet
    Dim R As Long, C As Long

    With ActiveCell
        Set Ws = .Worksheet
        R = .Row
        C = .Column
    End With

    With Ws
        If Len(Trim(.Cells(R, C - 2).Value)) = 0 Then
            'Go up find contract number copy
            Do Until Len(.Cells(R, C - 2).Value)
                R = R - 1
            Loop
            .Cells(R, "M").Copy Destination:=.Cells(ActiveCell.Row, "V")
        End If
    End With
End Sub

I think the ActiveCell component in this code is still a source of great danger. However, as you see, at least the code doesn't change it which dispenses with the necessity of going back to it in the end.

score 0 · Accepted Answer · answered Feb 14 '18 at 11:35

the issue lays in your keeping relying on ActiveCell after

ActiveCell.Offset(-1, 0).Select

statement, that changes it ...

you're actually playing with fire when using ActiveCell together with Select/Selection coding pattern!

since I cannot see what's behind the code you showed, I must keep using ActiveCell reference and amend your code as per comments:

Dim cellToCopy As Range
With ActiveCell 'reference currently "active" cell
    If .Offset(0, -2) = "" Then 'if the cell two columns left of referenced (i.e. "active") cell is empty...
        Set cellToCopy = .Offset(0, -2).End(xlUp) '... set cell to copy as the first not empty one above the cell two columns left of referenced (i.e. "active") cell
    Else '... otherwise
        Set cellToCopy = .Offset(0, -2) 'set cell to copy as the one two columns left of referenced (i.e. "active") cell
    End If
    cellToCopy.Copy Destination:=Range("V" & .Row) 'copy the cell set as the one to be copied and paste it column V cell same row as reference (i.e. "active") cell
End With

I'm sorry for such a late reply. Thank you it worked! I'll keep in mind not to use select and activecell reference together. — Tomek, Mar 02 '18 at 07:32
you are welcome. as for the `Selection/Active` issue you may want to read [This post](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba) — DisplayName, Mar 02 '18 at 07:37

Looping & copying cells

3 Answers3

You didn't select the desired cell