VBA + Copy data to new sheet while looping

September 11, 2010 at 19:23:28
Specs: Windows XP
I am trying to copy several rows of data from column A and D in worksheet 1 (Employee) to worksheet 4 (ADP) of the same workbook. The first row of data is copied to row 3, columns B & C of worksheet 4. The second row of data is copied to the same columns in worksheet 4 but eight lines down. The third row of data is copied to the same columns in worksheet 4 but eight lines further down. I was testing it on Column A first. So far this is what I have but it does not work. Can someone help?

Dim frstow As Range
Set frstow = Sheets("ADP").Range("C3")
With Sheets("Employee")
finalRow = .Cells(Rows.Count, 1).End(xlUp).Row
For x = 2 To finalRow
.Cells(x, 1).Copy
'nextrow = Cells(Rows.Count, 1).End(xlUp).Row (commented out/was testing)
nextrow = frstow.Offset(0, 3)
Cells(nextrow, 0).Select
'.Cells(x, 3).Paste (commented out/was testing)
nextrow = nextrow + 6
Next x
End With

See More: VBA + Copy data to new sheet while looping

Report •

September 12, 2010 at 10:35:44

A number of points:
1. You should always Dim all variables, and set them to the required variable type.

Dim frstow As Range
Dim finalRow As Integer
Dim nextRow As Integer
Dim x As Integer

2. It is helpful to name the variables starting with their type, so rngFrstRow or dblNextRow.
3. When referencing a cell, decide if you are referencing the cell or some part of the cell such as its Value or its displayed Text.
"frstow.Offset(0, 3)" is a cell. As you tried to make "nextRow" equal to it, Excel has done the next best thing - as it can't make the variable nextRow into a Cell object - it has taken the Value in the cell and made nextRow equal to it.
You likely intended nextRow to be equal to the Row value of that cell - so "nextRow = frstow.Offset(0, 3)" should have been "nextRow = frstow.Offset(0, 3).Row"
4.You can't use Cells on their own. You have to identify the worksheet object, so: Worksheets("ADP").Cells(nextRow, 0).Select is required.
4. This should have said: You shouldn't use Cells on their own. Identify the worksheet object: Worksheets("ADP").Cells(nextRow, 0).
Otherwise Cells refers to the active worksheet and, particularly when working on more than one worksheet, Cells on its own may refer to the wrong worksheet. (Thanks to DerbyDad03 for pointing out that what I posted was not right.)
5. In VBA you rarely need to Select something, to act on it.
6. There are two Copy/Paste methods that can be used which do not rely on the Active cell.
6a. One copies the whole cell to a destination cell - the whole cell object is copied and pasted:
Worksheets("Employee").Cells(x, 1). Copy Destination:= Worksheets("ADP").Range("A" & Cstr(nextRow))
Note that the cell's value is not referenced, as the whole cell is copied.
6b. Second a Paste Special that pastes certain parameters of a cell - including "All", "Values", "Formats" ...
It is done on two lines:
Worksheets("Employee").Cells(x, 1). Copy
Worksheets("ADP").Range("A" & Cstr(nextRow)).PasteSpecial Paste:=xlPasteValues
7. Use Option Explicit before the first line of code
This reduces the risk of simple Typos
nextrow =frstrow+1
This would be run with no errors, but the range would be "A1" and not "A5" as expected.
8. I have a preference when looping through a range of cells to use VBA's ability to enumerate all the items in a group. So a group (a range in this example) of cells can be looped through like this:
Dim rngCell as Range
Dim rngSource as Range
Set rngSource = Worksheets("Employee").Range("A2:A20")
For Each rngCell in rngSource
'.... rngCell is each cell in the range A2 to A20 in turn
Next rngCell
- so there is no need for a control counter (x in your code)

Here is what I wrote:

Option Explicit

Sub MoveByEights()
Dim rngCell As Range
Dim rngStart As Range
Dim rngEnd As Range
Dim rngSource As Range
Dim rngDestStart As Range
Dim intDestRowOffst As Integer

'set start of Source data in column A
Set rngStart = Worksheets("Employee").Range("A2")
'find end of source data in column A
Set rngEnd = Worksheets("Employee").Range("A" & CStr(Application.Rows.Count)) _
'create source data range
Set rngSource = Range(rngStart, rngEnd)
'set start of destination data in column B
Set rngDestStart = Worksheets("ADP").Range("B3")
'set destination row offset counter to zero
intDestRowOffst = 0

'loop through source data in column A
For Each rngCell In rngSource
    'copy cell in column A to destination column B
    rngCell.Copy _
        Destination:=rngDestStart.Offset(intDestRowOffst, 0)
    'copy cell in column D to destination column C
    rngCell.Offset(0, 3).Copy _
        Destination:=rngDestStart.Offset(intDestRowOffst, 1)
    'increment destination row offset by 8
    intDestRowOffst = intDestRowOffst + 8
Next rngCell
End Sub

You can see that some steps I use are not strictly necessary, for instance initializing the destination row counter to zero - this is the default in VBA
and converting the number of rows in the find end of data line from an integer to a string using Cstr(Application.Rows.count); as VBA will 'coerce' the value from an integer to the required string automatically.


Report •
Related Solutions

Ask Question