How do I bring up a record based on a combobox in Access?

In commenting on @Remou's perfectly valid and helpful answer, I alluded to the fact that the find combo box wizard creates really bad code. Here's the code the wizard creates when you choose an Autonumber PK for the bound column (there is slight variation in the code the wizard creates if you're searching on a text field instead of numeric, but it's not enough to mention):

  Private Sub Combo2_AfterUpdate()
    ' Find the record that matches the control.
    Dim rs As Object

    Set rs = Me.Recordset.Clone
    rs.FindFirst "[InventoryID] = " & Str(Nz(Me![Combo2], 0))
    If Not rs.EOF Then Me.Bookmark = rs.Bookmark
  End Sub

One thing that's wrong with it is that you can't run it on an existing control, so you end up with a randomly named combo box, and when you change the combo box's name, you have to reapply it to the event, and edit it to reflect the change of name. But that's relatively minor compared to the other problems in the wizard code itelf, which has an error rate of at least 2.5 problems per line of code created.

Here's my alternative code for it:

  Private Sub cmbFind_AfterUpdate()
    If IsNull(Me!cmbFind) Then Exit Sub

    With Me.RecordsetClone
      .FindFirst "[InventoryID] = " & Me!cmbFind
      If Not .NoMatch Then
         If Me.Dirty Then Me.Dirty = False
         Me.Bookmark = .Bookmark
      Else
         ' put your not found code here, but you really shouldn't need it
      End If
    End With
  End Sub

First off there is absolutely no reason to define a recordset variable of any type at all because you can easily operate on the appropriate recordset directly.

Second, if you do declare it, it's really rather defensive programming to declare it as an Object variable. Given that .FindFirst works only on a DAO recordset, it's always going to be a DAO recordset that is the only recordset type that the rest of the code can work on (whether or not the form's Recordset object is always a DAO recordset -- I'm not even certain that's true). So using an Object type variable is only necessary if you're operating without a DAO reference in your application.

This seems overly cautious, but my main point is that there's no reason to declare a variable in the first place.

Third, if you do assign a recordset to the variable, you need to clean up after yourself and set the variable to Nothing at the end of the sub, and close the clone of the form's recordset that you created.

Fourth, there is no reason to use a clone of the form's recordset because the RecordsetClone already exists, and its whole reason for existing is precisely for this kind of usage.

Fifth, the handling of a Null value in the combo box is crazy -- going ahead and cloning a recordsource even when you aren't going to find anything makes no sense to me. If it's Null, just exit the sub (or create a label for the exit point and jump to that), rather than going through the trouble of cloning the recordset and doing a FindFirst operation that can be known to be fruitless.

Sixth, FindFirst is not efficient -- it does a sequential scan through the field's index, or through the table itself if there's no index -- so you want to avoid initiating one if you don't need to in the first place.

Seventh, using Nz() to return 0 if the combo box is Null will produce incorrect results if 0 is actually a valid value for the field being searched.

Eighth, doing the FindFirst even when you deleted the value from the find combo box moves the current record back to the first one, and the logical behavior would instead be to leave the current record wherever it was in the first place before you deleted the value from the find combo box. That is, if you're not searching, don't find something!

Ninth, using EOF as your test assumes that the FindFirst does a table scan rather than an index scan (I don't know that it does or doesn't), and that the FindFirst moves the pointer in the cloned recordset even if there are no results (as opposed to when there are none).

Tenth, why use EOF when every recordset has a NoMatch property precisely for this purpose and no other? There are no ambiguities about what it means when tested after a FindFirst command, unlike EOF, which reports whether the record pointer has reached the end of the table or not. One property, NoMatch, has a narrow meaning and can't mean anything else, and exists precisely for use after a FindFirst operation, while EOF has a much broader meaning that is being used as a proxy here for something else.

Eleventh, and the most serious defect, is that the wizard code does not explicitly force a SAVE if the record is dirty before setting the bookmark. This is a crucial mistake, as this is an area in which Access has been unreliable over the years -- errors that happen from the implicit save initiated by departing the initial record by setting the bookmark can be lost and result in lost data. Theoretically, that is a bug that was fixed a long time ago, but explicitly forcing the save before navigating to another record is best practice, as you're allowing any errors in the save operation to happen indendently from the navigation operation.

Need I say more?

Why is it this way? My first guess would have been that the wizard produces the same code in an MDB/ACCDB and in an ADP, but ADP forms can't return DAO recordsets, so you wouldn't have FindFirst available. Perhaps in an ADP it uses Find instead of FindFirst. That would explain why EOF is used instead of NoMatch, since ADO recordsets lack NoMatch.

But why should my MDBs/ACCDBs be crippled by the requirements of an ADP, which has nothing to do with them? And if I'm right that there is conditional code for determining whether to use Find or FindFirst, then why not go whole hog and use the most appropriate methods in the context in which the wizard is run?

This is terrible code, and needs to be rewritten any time the wizard is called. It could have been better code, but for some unknown reason, MS chose to produce botched code. This is quite in contrast to the code produced by all the other Access wizards I've ever used -- I may find them a bit over-verbose in some cases, but there's good justification for that in terms of extensibility. I simply cannot comprehend the reason why this particular wizard produces such awful code.


You can use the wizard to add a combobox to your bound form. One of the options the wizard offeres is "Find a record on my form based on the value I select in my combobox". If your form is complex, you may not see this option, in which case create a simple form to see the wizard and generate sample code - not always the best code, but certainly very useful.

There are a few small things that may or may not trouble you, for example, if the user moves to a record without using the combo, the combo does not change to show the new record, but this is very easy to fix with a little code in the current event for the form.