Transfer of data instead of copy.paste.clear used

Question

I am using this code in a worksheet to check the condition, and based on the condition, copying the entire row of values in a different range of particular condition if true.

For copying I am clearing the destination range each time and copying the same data with updated information on the same destination. This makes the code running longer. Can I make the code to copy only the updated cells and merge onto the destination ranges where the old data exists?

Private Sub Worksheet_Change(ByVal Target As Range)
Application.ScreenUpdating = False
If (Target.Row >= 120 And Target.Row < 1400) And (Target.Column >= 1 And Target.Column <= 4) Then
    Dim rngSinglecell As Range
    Dim rngQuantityCells As Range
    Dim intCount As Integer

    Sheets("CE1 Data").Range("K2:S1400").Clear

    Set rngQuantityCells = Sheets("CE1 Data").Range("B120", Range("B120").End(xlDown))

    For Each rngSinglecell In rngQuantityCells

    If rngSinglecell.Value = "Primary" Then

    Range(Range("A" & rngSinglecell.Row), Range("D" & rngSinglecell.Row)).Copy Range("K" & Rows.Count).End(xlUp).Offset(1)

    ElseIf rngSinglecell.Value = "Backup" Then

    Range(Range("A" & rngSinglecell.Row), Range("D" & rngSinglecell.Row)).Copy Range("P" & Rows.Count).End(xlUp).Offset(1)

    End If
Next

End If

Application.ScreenUpdating = True
Application.CutCopyMode = False
End Sub

Answer 1

I think apart from the performance your code also has some other issues. Let me first come to the most problematic, then make some suggestions regarding the performance, and finally make some general comments.

The biggest problem I see here is that procedure also copies functions. This can be very problematic if the references within the formulas are not absolute references. Then they will point somewhere totally different than before.

From the point above, I guess that you are only intending to copy values. In this case, you can avoid using the the rather slow copy function and simply copy the values accessing them via the Value or Value2 property of the cells. (The latter is faster and behaves more reliably, see this question on Stack Overflow.)

To speed up things even somewhat more, you can read the entire range you like to copy into an array via Value2 and then process it. You can also set entire ranges from arrays by setting the Value property of a range. Using arrays is the fastest thing you can do to copy large ranges. (See this blog post testing the various ways to copy the values in ranges in Excel.)

Now, the general comments on the code:

1. The use of Target in the procedure, or at least its name, is rather confusing. Apparently, it is only used in the condition that is tested and it is never used as a target to copy or the like. It seems as if the check might belong into another procedure calling this one.

2. Your code will throw an error whenever Sheet(CE1 Data) is not the active sheet. The problem is that in:

   Set rngQuantityCells = Sheets(CE1 Data).Range(B120, Range(B120).End(xlDown))
   

   the second Range is not qualified, i.e. it is always implicitly referring to the active sheet. It would be a good practice to always qualify calls to Range, even if it's from ActiveSheet.

3. If the sheet Sheet(CE1 Data) is not active, the paste range will not be inside the cleared range because the paste range will be on the active sheet. (This obviously only applies after qualifying the offending Range from the second comment.)

4. You might want to save the worksheet in a variable and not query the Sheets collection all the time. Personally, I would even inject the worksheet to use as a parameter of type Worksheet.

5. Given that you apparently only use this procedure from the worksheet Sheet(CE1 Data), you might consider using a With statement to access the sheet.

6. You are using quite a number of magic numbers in the procedure, like the bounds in which Target may be or the explicit ranges. It might be better to use constants or named ranges. If you ever want to change from where this procedure reads or to where it writes, named ranges would make your life easier.

Finally, you might consider to remove most blank lines and indent according to nesting like in:

For Each rngSinglecell In rngQuantityCells
    If rngSinglecell.Value = Primary Then
        Range(Range(A & rngSinglecell.Row), Range(D & rngSinglecell.Row)).Copy Range(K & Rows.Count).End(xlUp).Offset(1)
    ElseIf rngSinglecell.Value = Backup Then
        Range(Range(A & rngSinglecell.Row), Range(D & rngSinglecell.Row)).Copy Range(P & Rows.Count).End(xlUp).Offset(1)
    End If
Next

EDIT:

I did not really realize that you wrote a handler for the worksheet changed event. So, the parameter Target has to be there. However, it would be good practice to separate the two current responsibilities of your handler, to decide what kind of thing to do on a worksheet changed event and to copy over the cells. This can be achieved by extracting everything inside the conditional into its own sub, which preferably should not depend on the active sheet. Then my comments above apply to that sub.

One more thing: Your idea to change only the data that changed has a few issues. Whenever you change a value in column B, you have to rewrite everything because rows will potentially drop out or enter one of the two target lists. Moreover, the Target of the worksheet changed event can contain more then one cell. In this case it might actually be faster to simply copy everything instead of figuring out which row to change in the target lists for every cell in Target.

Considering that Target may contain more than one cell, you might also want to consider whether your condition on Target is correct in all circumstances.