0

I have a Macro that takes a selection of cells from "New Search" and paste the range to the first open cell in "Past Searches" I'm thinking that my code is very ineffective. Does anyone have any ideas of how I can improve this? 

Sub Macro5()
Range("A3:J3").Select
Range(Selection, Selection.End(xlDown)).Select
Selection.Copy
Sheets("Past Searches").Select
Range("A1").End(xlDown).Offset(1, 0).Select
ActiveSheet.Paste
Worksheets("New Searches").Activate
Application.CutCopyMode = False
End Sub
Community
  • 1
  • 1
Taylor Zwick
  • 27
  • 2
  • 3
  • 10
  • 1
    Your code can be cleaner and more efficient if you get rid of `.Select` and `.Activate`. Read the note at the end of this answer for how to go about that: [Copy Paste Macro...](http://stackoverflow.com/a/17929211/138938) – Jon Crowell Jul 31 '13 at 14:01
  • To add to what @HeadofCatering has said, check out [this](http://stackoverflow.com/questions/10714251/excel-macro-avoiding-using-select/10718179#10718179) answer which has methods to avoid `Select` – SeanC Jul 31 '13 at 14:10
  • @SeanCheshire, the Copy Paste Macro... answer also links to the question Sid answered. (Great minds!) Chris Nielsen's answer (the accepted answer) also has lots of useful info. – Jon Crowell Jul 31 '13 at 16:01

3 Answers3

3

First, you don't have to use .Select or .Activate on cells/worksheets that you'll manipulate, just reference them directly. That can really slow down your code.

The .Copy method can take a Destination parameter, so you don't have to use .Paste somewhere else.

Also, the .End property isn't very reliable. It's better to use UsedRange instead.

You can do that pretty much in only 1 line.

Worksheets("New Search").Range("A3:J3").Copy Destination:=Worksheets("Past Searches").UsedRange.Columns(1).Offset(1, 0)
Fabio Pereira
  • 338
  • 1
  • 8
  • Thanks for pointers, but it seems like your code is only selecting `A3:J3` and not the rows underneath the first row? – Taylor Zwick Jul 31 '13 at 14:26
0

Fabio thanks for pointing me in the right direction, I wouldn't have been able to turn that mess in to concise code.

numofrows = ActiveSheet.UsedRange.Rows.Count`

Worksheets("New Searches").Range("A3", "J" + CStr(numofrows)).Copy Destination:=Worksheets("Past Searches").Range("A1").End(xlDown).Offset(1, 0)

This ended up doing what I need to do.

Taylor Zwick
  • 27
  • 2
  • 3
  • 10
0

You could also do this so you don't need numofrows?

Worksheets("New Searches").Range(Range("A3:J3"), Range("A3:J3").End(xlDown)).Copy Destination:= _
Worksheets("Past Searches").Range("A1").End(xlDown)
MakeCents
  • 742
  • 1
  • 5
  • 15