Leon
Leon

Reputation: 97

Excel VBA Error handling not doing its job

So, I will preface that I am still very much a novice in VBA. I typed this code up the best that I could and was proud. I decided to add some extra features. The very last feature I wanted was to capture specific information when there was an error and display them in a message box.

What the code does:

I have two workbooks. Wkbook1 is filled with tons of data elements and Wkbook2 is the master list. The code does a search for the first line in wkbook1 and does a search for that phrase in Wkbook2 and then initials to the left of it. Sometimes, there are repeating elements and my "DO LOOP" takes care of that. My problem is that not all elements are present in the master list and I have to know which ones aren't.

I set it up so that when there is an error message because it couldn't find the element in the master list, It went to my error handler and stored the elment into my message variable to be used later with the message box.

I tested it on other data element workbooks and if there are 0 or 1 elements not found it works, but when there are more than 1 elemets not found in the master list, it gives me run-time error 91 on the second element not found. I'm sure there are plenty of critiques on my code, so please take it easy on me. This is my first time using arrays.

Code:

Option Explicit

Sub M2_Name_Finder()

Dim x As String
Dim y As Integer
Dim z As Integer
Dim c As Integer
Dim m As Integer
Dim name As String
Dim rngCopy As Range
Dim numRows As Long
Dim rAddress As String
Dim found(50) As String
Dim previous As String
Dim missingFields As String
Dim message As String

name = "JJP"

Windows("Wbook1").Activate
Range("D3").Select

Set rngCopy = ActiveCell.CurrentRegion
numRows = rngCopy.Rows.Count

For z = 1 To numRows

  Windows("Wkbook1").Activate
  Range("D3").Select
  ActiveCell.Offset(y, 0).Select
  x = ActiveCell.Value

  If x = vbNullString Or x = " " Then
     GoTo Done
  End If

  If x = previous Then
     GoTo Here
  End If

  previous = ActiveCell.Value
  Windows("Wkbook2").Activate
  Columns("D:D").Select

  On Error GoTo Missing:
  Selection.Find(what:=x, After:=ActiveCell, LookIn:=xlFormulas, LookAt _
     :=xlWhole, SearchOrder:=xlByColumns, SearchDirection:=xlNext, MatchCase _
     :=False, SearchFormat:=False).Activate

  found(c) = ActiveCell.Address
  rAddress = ActiveCell.Address

  If ActiveCell.Value <> Empty Or ActiveCell.Value <> 0 Then

     Do
        ActiveCell.Select
        ActiveCell.Offset(0, -3).Value = name
        c = c + 1
        Cells.FindNext(After:=ActiveCell).Activate
        found(c) = ActiveCell.Address
     Loop While found(c) <> rAddress

  End If

  Here:
  c = 0
  y = y + 1

Next z

Missing:
message = message & x
m = m + 1
GoTo Here:

Done: MsgBox "Not found: " & message & vbLf, vbInformation

End Sub

Upvotes: 2

Views: 4866

Answers (2)

Tony Dallimore
Tony Dallimore

Reputation: 12413

I approve that you have included error handling and I have no criticism of mischab1's answer.

However, I do not approve of your use of error handling to catch an error you are expecting; its there for the unexpected. The following code is much better:

Dim Rng As Range
:
:
Set Rng = .... Find ....
If Rng Is Nothing Then
  ' The Find has failed to locate the required string
  ' Include code for this situation
Else
  ' The Find has found the required string
  ' Include code for this situation
End If

You still have your error routine for the unexpected but with this code it is not used for the routine situation of a failed Find.

You asked us to be gentle with you and mischab1 respected your wish but I am going to raise one issue.

If you switch between workbooks and worksheets and use select and activate, your code will be sloooow because the screen is being continually repainted. You can increase the speed by including:

Application.ScreenUpdating = False

at the beginning and

Application.ScreenUpdating = True

at the end.

There will still be a flash when you switch workbooks but most of the screen activity will be avoided.

However, it would be better to eliminate Select and Activate as well. Look through the last month or two's questions and answers. Many (perhaps most) will contain nothing of interest but many of the questions come from beginners and some answers include excellent code for handling multiple worksheets and workbooks and copying data from one to the other. I believe studying these answers will quickly repay your study time.

Happy programming!

Upvotes: 4

mischab1
mischab1

Reputation: 1601

Yes, there are a lot of things I would suggest changing about your function. For now though I'll just touch on the specific problem you are having. :-)

The problem is that the first time you enter error handling, when you use GoTo to go back to the regular code, you haven't left the error handler. So the 2nd time you get an error it raises up out of the error handler to the user. You need to use Resume instead. Resume tells the system that you are done handling the error and returning to regular code execution.

Missing:
   message = message & x
   m = m + 1
   Resume Here:

Upvotes: 4

Related Questions