1

I am trying to write a script which checks for duplicate values in another worksheet, but I cannot get it to work. At line problem the If function always proceeds, whether set to If Not or If. LocatedCell does equal Nothing.

I am sure this is an obvious error but I cannot understand it.

Sub mailer_followuptest()

Application.ScreenUpdating = False

'Remove matching contacts data from last run
   Dim wsDel As Worksheet
    Application.DisplayAlerts = False
    Err.Clear
    On Error Resume Next
    Set wsDel = Sheets("Matching Contacts")
    wsDel.Delete

Dim mailerSheet As Worksheet
Set mailerSheet = Worksheets("Call data")

Set MatchingContacts = Sheets.Add
MatchingContacts.Name = "Matching Contacts"

Dim DesiredEntry As String
Dim CRMContacts As Worksheet
Set CRMContacts = Worksheets("CRM contacts")

CRMContacts.Select
Range("A1").Select

Do
ActiveCell.Offset(1, 0).Select
DesiredEntry = ActiveCell.Value


    With Sheets(mailerSheet).Range("A:A")

        Dim LocatedCell As Range
        Set LocatedCell = .Find(What:=DesiredEntry, SearchOrder:=xlByRows, LookAt:=xlPart)



problem: If Not LocatedCell = "Nothing" Then

             'With_
             LocatedCell.EntireRow.Copy_
                 '.Interior.ColorIndex = 4 'green
             'End With

        MatchingContacts.Select
        Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
            :=False, Transpose:=False
        Application.CutCopyMode = False
        ActiveCell.Offset(1, 0).Select


        End If

    End With

    CRMContacts.Select

Loop Until ActiveCell.Value = ""

Application.ScreenUpdating = True


End Sub

Additionally, am I using find correctly? It doesn't appear to be working either.

Cassiopeia
  • 313
  • 1
  • 4
  • 16
  • 1
    The way you use `Nothing` is not proper. It should go with `Is` and not the `=` sign. Try this instead `If Not LocatedCell Is Nothing Then` or `If LocatedCell Is Nothing Then`. Also, the way you use `.Find` has many holes. Try adapting to [THIS](http://www.cpearson.com/excel/findall.aspx) – L42 Feb 10 '14 at 09:42
  • 1
    One tip: Using `On Error Resume Next` means telling the code to "Shut UP" and do what you want. In most cases it will do what you want... Shut Up and perform... but then you will not get the expected results. So please use `OERN` judiciously ;) – Siddharth Rout Feb 10 '14 at 10:17

1 Answers1

12

Use On Error Resume Next judiciously.

Don't use On Error Resume Next for the entire code - it will hide all your errors. Use it only when it's really needed.

Using On Error Resume Next means telling the code to Shut UP and do what you want. In most cases it will do what you want... Shut Up and perform... but then you will not get the expected results or totally wrong results as shown below !!! (SiddharthRout ©:)

enter image description here

Change

Err.Clear
On Error Resume Next
Set wsDel = Sheets("Matching Contacts")
wsDel.Delete

to

On Error Resume Next
Set wsDel = Sheets("Matching Contacts")
On Error GoTo 0
If Not wsDel Is Nothing Then wsDel.Delete

line On Error GoTo 0 will return your error handler to default mode.

Some issues with your code:

1) In line If Not LocatedCell = "Nothing" Then you tries to identify whether your cells value doesn't equal string "Nothing" which is uncorrect.

To check whether the .Find function returns any cell, change 

If Not LocatedCell = "Nothing" Then

to 

If Not LocatedCell Is Nothing Then

2) change With Sheets(mailerSheet).Range("A:A") to With mailerSheet.Range("A:A")

3) as @SiddharthRout mentioned in comments below,

The copy has to be right before the paste special. Excel is very well known for clearing the clipboard if you perform some specific actions

if you are going to change interior color and copy row, change 

'With_
   LocatedCell.EntireRow.Copy_
   '.Interior.ColorIndex = 4 'green
'End With

to

With LocatedCell.EntireRow
     .Interior.ColorIndex = 4 'green
     .Copy
End With

4) and of course: How to avoid using Select/Active statements

Community
  • 1
  • 1
Dmitry Pavliv
  • 35,333
  • 13
  • 79
  • 80
  • 2
    + 1 Esp for the last line. Also I see one more problem. The copy has to be right before the paste special. Excel is very well known for clearing the clipboard if you perform some specific actions. For example `.Interior.ColorIndex` And not to mention the biggest problem of all! Using `On Error Resume Next` recklessly ;) – Siddharth Rout Feb 10 '14 at 10:15
  • Very tempted to add a cartoon to your post which I just created :D – Siddharth Rout Feb 10 '14 at 10:35
  • You can undo the changes if you want to :P – Siddharth Rout Feb 10 '14 at 10:40
  • Thanks, I always wondered how to return to the default error handling. ~ When I set `If Not LocatedCell = Nothing Then` i get "Invalid use of object and it highlights `Nothing` ~ @L42 points out it should be The way you use Nothing is not proper. It should go with Is and not the = sign. Try this instead `If Not LocatedCell Is Nothing Then` or `If LocatedCell Is Nothing Then`. – Cassiopeia Feb 10 '14 at 11:01
  • You should use `Is` keyword instead `=` sign. *Correct statement:* `If Not LocatedCell Is Nothing Then`. *UNcorrect statement:* `If Not LocatedCell = Nothing Then` – Dmitry Pavliv Feb 10 '14 at 11:07
  • 3
    haha never thought that @SiddharthRout is a cartoonist.LOL :D – L42 Apr 14 '14 at 07:28