1

If you guys could help me out, that would be great because it would really help me.

Here's what I'm trying to do:

  1. Search for a cell with a specific term
  2. If found, copy the entire row that the cell is in and paste it into a row above it.
  3. If not found, do nothing and continue with the code

Here's my code:

Sub Test()
'
' Test Macro
'
' Keyboard Shortcut: Ctrl+b
'

    Range("A5").Select
    Cells.Find(What:="PL 1", After:=ActiveCell, LookIn:=xlFormulas, LookAt _
        :=xlPart, SearchOrder:=xlByRows, SearchDirection:=xlNext, MatchCase:= _
        False, SearchFormat:=False).Activate
    If Not IsEmpty(ActiveCell.Value) Then
        ActiveCell.Rows("1:1").EntireRow.Select
        Selection.Copy
        Range("A5").Select
        ActiveSheet.Paste
    End If
    Range("A5").Select
    Cells.Find(What:="PL 2", After:=ActiveCell, LookIn:=xlFormulas, LookAt _
        :=xlPart, SearchOrder:=xlByRows, SearchDirection:=xlNext, MatchCase:= _
        False, SearchFormat:=False).Activate
    If Not IsEmpty(ActiveCell.Value) Then
        ActiveCell.Rows("1:1").EntireRow.Select
        Selection.Copy
        Range("A6").Select
        ActiveSheet.Paste
    End If
    Range("A5").Select
    Cells.Find(What:="PL 3", After:=ActiveCell, LookIn:=xlFormulas, LookAt _
        :=xlPart, SearchOrder:=xlByRows, SearchDirection:=xlNext, MatchCase:= _
        False, SearchFormat:=False).Activate
    If Not IsEmpty(ActiveCell.Value) Then
        ActiveCell.Rows("1:1").EntireRow.Select
        Selection.Copy
        Range("A7").Select
        ActiveSheet.Paste
    End If
End Sub

My code only works if the value is found. If it's not found it runs into the error below: enter image description here

GT.
  • 764
  • 1
  • 8
  • 30
  • 2
    "It's not working, obviously" You need to tell us what it's doing, over what specific data, and what you expect it to do. Not only are you asking for SO to debug this for you, you're asking for debugging to be done blindly, without even knowing what your source data looks like. – Grade 'Eh' Bacon May 31 '16 at 15:51
  • 1
    Also, this has been asked hundreds of times on SO. Good on you to write your own code but why reinvent the wheel? – findwindow May 31 '16 at 15:53
  • @findwindow do you have a link? I can't find it. – GT. May 31 '16 at 15:55
  • @Grade'Eh'Bacon I appreciate your passion. – GT. May 31 '16 at 15:57
  • 1
    http://stackoverflow.com/search?q=search+string+copy+row – findwindow May 31 '16 at 15:58
  • @findwindow Thanks for the link. I can actually search and copy just fine if my search term is found in the sheet. My problem is that my search term isn't always in my sheet and I need some way to pass over the search term and move on to the next one. – GT. May 31 '16 at 16:00
  • So add an `if`statement? – findwindow May 31 '16 at 16:02
  • As you can see in my code, I attempted to do just what you suggested but I failed. That's why I'm asking for help. – GT. May 31 '16 at 16:05
  • Oh, just saw the edit XD – findwindow May 31 '16 at 16:08
  • @GeorgeTye : http://stackoverflow.com/questions/30161124/vba-find-and-adding-a-value/30162390#30162390 : Here is an example of the structure if you want to keep searching after doing your actions! ;) – R3uK May 31 '16 at 16:08

2 Answers2

3

You can try something like this instead (untested):

Sub HideAndSeek()

    Dim foundCell As Range

    For i = 1 To 3
        Set foundCell = Cells.Find(What:="PL " & i, LookIn:=xlFormulas, LookAt:=xlPart)
        If Not foundCell Is Nothing Then
            Intersect(foundCell.EntireRow, ActiveSheet.UsedRange).Offset(-1, 0).Value = _
                Intersect(foundCell.EntireRow, ActiveSheet.UsedRange).Value
        End If
        Set foundCell = Nothing
    Next

End Sub

The principle being that you write the code you need once and then create a loop to repeat the code for you.

The other part of this answer is checking that the cell was found - to do this we check that the range was actually set (which means it isn't Nothing) using

If Not foundRange Is Nothing
SierraOscar
  • 17,507
  • 6
  • 40
  • 68
  • Thanks for being so helpful @Macro Man! I'll try this. I think it might have been difficult for the other people to see what the issue was, so it helps to have someone like you. – GT. May 31 '16 at 16:09
  • I'm having a little trouble understanding your code. Why did you use Intersect()? What's the advantage of that over simple copy & paste? – GT. May 31 '16 at 16:16
  • `Intersect()` finds returns the section where 2 ranges overlap (or _intersect_). In general it's better to directly write the value of a range to another range rather than copy/paste (cleaner and less overhead). Rather than do that for the _entire_ row which is overkill I just used `Intersect` to find where the whole row and the used range overlap, and then write that value to the same area but one row above using `Offset(-1, 0)` – SierraOscar May 31 '16 at 16:21
  • Oh ok thanks. What if I want it to write to a specific row? Is that possible? – GT. May 31 '16 at 16:26
3

Cells.Find is a function that returns a Range object reference; when it doesn't find anything, the reference will be Nothing. And you can't call .Activate on Nothing:

This method returns Nothing if no match is found. The Find method does not affect the selection or the active cell. (MSDN)

Cells.Find(What:="PL 2", After:=ActiveCell, LookIn:=xlFormulas, LookAt _
    :=xlPart, SearchOrder:=xlByRows, SearchDirection:=xlNext, MatchCase:= _
    False, SearchFormat:=False).Activate

You need to rewrite your code and avoid .Select and .Activate, and avoid working with ActiveCell and implicitly with ActiveSheet (which you are doing by not qualifying the Cells call with a proper worksheet reference).

Your formatting makes it hard to read the code, for several reasons:

  • Arguments are being specified on different lines
  • Line continuations are being palced at arbitrary locations
  • Nested member calls aren't lined up

Compare to:

Cells.Find(What:="PL 2", After:=ActiveCell, LookIn:=xlFormulas, _
           LookAt:=xlPart, SearchOrder:=xlByRows, SearchDirection:=xlNext, _
           MatchCase:=False, SearchFormat:=False) _
     .Activate

That's just readability. The problem is that you basically assume that .Find returns a valid object reference. Don't assume, explicitly check:

Set result = Cells.Find(...)
If Not result Is Nothing Then result.Activate

But really, you need to figure out a way to avoid .Select and .Activate.

Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
  • 1
    Wow. Perfect answer. This is exactly what I was looking for. Now I understand where I was wrong much better. – GT. May 31 '16 at 17:43
  • So why are .Select and .Activate bad? – GT. Jun 01 '16 at 13:15
  • Because they make your code extremely brittle, harder to follow, and uselessly slow. See [this question](http://stackoverflow.com/q/10714251/1188513) for *how* to avoid them, and [this MSDN article](https://msdn.microsoft.com/en-us/library/aa139976(v=office.10).aspx) as well as [this blog post](http://www.businessprogrammer.com/power-excel-vba-secret-avoid-using-select/) and [this Office blog entry about VBA best practices](https://blogs.office.com/2009/03/12/excel-vba-performance-coding-best-practices/), for the *why*. – Mathieu Guindon Jun 01 '16 at 13:54
  • Thanks so much Mat's Mug. Geez Mat must have been a really talented potter to come up with something (someone?) as helpful as you. – GT. Jun 01 '16 at 14:44
  • I wrote some code that is sufficient for what I need to do. Sure, it is a little slow but it's still 1000x faster than doing it manually. However, I will definitely read through the materials you suggested because I am always looking to improve. – GT. Jun 01 '16 at 14:45
  • Thanks! Feel free to come over to [codereview.se] with your working code looking for improvement =) – Mathieu Guindon Jun 01 '16 at 14:46
  • Oh wow. Stack Overflow has everything doesn't it? That sounds like a great idea. I actually made a project in Javascript that I would be interested in getting reviewed as well. I will definitely be headed there at some point in the future. – GT. Jun 01 '16 at 14:49