1

To allow portability of an Access database, I want to force the user to select its folder if it's copied to another computer. I have run into a stumbling block, though, in trying to test for the folder's path.

In the code below, the if statement block works when not commented out, but the while statement above it does not. I get:

Run-time error '5': Invalid procedure call or argument.

I've looked at Tools > References, and the appropriate pieces seem to be in place. I've tried fd.SelectedItems.Count = 0, but that doesn't prevent an unwanted string from being passed. 

Private Sub btn_CorrectPath_Click()
   Dim sHostName As String, strSQL As String, sFolder As String
   Dim rs As Recordset, db As Database, fd As FileDialog
   Dim intResult As Integer

   Set db = CurrentDb
   ' Get Host Name / Get Computer Name
   sHostName = Environ$("computername")
   Set rs = CurrentDb.OpenRecordset("SELECT * FROM t_ComputerInfo")

   If rs!ComputerName <> sHostName Then
      Set fd = Application.FileDialog(msoFileDialogFolderPicker)
      fd.AllowMultiSelect = False
      fd.Title = "Select database folder"

      intResult = fd.Show
      While intResult = False
         intResult = fd.Show
         While fd.SelectedItems(1) = vbNullString 'folder path was not selected
            intResult = fd.Show
         Wend
      Wend
      sFolder = fd.SelectedItems(1)
      strSQL = "UPDATE t_ComputerInfo SET [t_ComputerInfo].[ComputerName] = '" & sHostName & _ 
             & " [t_ComputerInfo].[DBPath] = '" & sFolder & "' WHERE [t_ComputerInfo].[ID] = 1"
      CurrentDb.Execute strSQL, dbFailOnError

'      If fd.Show = True Then 'Action button was pressed
'         MsgBox ("Directory was given. fd.SelectedItems(1)= " & fd.SelectedItems(1))
'         If fd.SelectedItems(1) <> vbNullString Then
'            sFolder = fd.SelectedItems(1)
'            strSQL = "UPDATE t_ComputerInfo SET [t_ComputerInfo].[ComputerName] = '" & sHostName & _
                     "', [t_ComputerInfo].[DBPath] = '" & sFolder & "' WHERE [t_ComputerInfo].[ID] = 1"
'            MsgBox ("SQL statement = " & vbCrLf & strSQL)
'            CurrentDb.Execute strSQL, dbFailOnError
'         End If
'      Else 'Cancel button was pressed
'         sFolder = fd.SelectedItems(1)
'         MsgBox ("The location of the database is required and will be requested later. fd.SelectedItems(1)= " & sFolder)
'      End If
      Set fd = Nothing
   End If

   db.Close
End Sub
Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
N.Barrett
  • 61
  • 7
  • Which exact line causes the error? [Debugging VBA Code](http://www.cpearson.com/excel/DebuggingVBA.aspx) – Andre Dec 29 '19 at 08:38
  • `While fd.SelectedItems(1) = vbNullString` causes the error. – N.Barrett Dec 30 '19 at 03:31
  • 2
    I'm not sure what you are trying to do with this code, but are you aware that `CurrentProject.Path` gives you the current db's path? There is no need for the user to supply that path. – Andre Dec 30 '19 at 11:32
  • I've seen that before but never connected the dots. Thanks for pointing me to that; it'll help reduce the complexity of what I'm trying to do. – N.Barrett Dec 30 '19 at 11:42

2 Answers2

2

Firstly, note that you have doubled-up the ampersand in this concatenation:

strSQL = "UPDATE t_ComputerInfo SET [t_ComputerInfo].[ComputerName] = '" & sHostName & _
         & " [t_ComputerInfo].[DBPath] = '" & sFolder & "' WHERE [t_ComputerInfo].[ID] = 1"
         ^----- Here

For the rest of your code, I believe you can remove the while loop entirely (unless you really want the user to be stuck in a loop until they select a folder...?).

I might suggest something along the lines of the following:

Private Sub btn_CorrectPath_Click()
    Dim fdr As String
    Dim pcn As String

    pcn = Environ$("computername")
    If Nz(DLookup("computername", "t_computerinfo"), "") <> pcn Then
        With Application.FileDialog(4)
            .AllowMultiSelect = False
            .Title = "Select Database Folder"
            If .Show Then fdr = .selecteditems(1)
        End With
        If fdr <> "" Then
            With CurrentDb.CreateQueryDef("", "update t_computerinfo t set t.computername = @pcn, t.dbpath = @fdr where t.id = 1")
                .Parameters(0) = pcn
                .Parameters(1) = fdr
                .Execute dbFailOnError
            End With
        End If
    End If
End Sub

Here, I've used DLookup in place of opening a recordset, as this seemed overkill when querying a single value, though, you will probably want to supply DLookup with some criteria.

I've also used a parameterised SQL statement in place of concatenating values, as this is better practice and also takes care of the data types.

June7
  • 19,874
  • 8
  • 24
  • 34
Lee Mac
  • 15,615
  • 6
  • 32
  • 80
  • Thanks for catching that editing error. Also, I didn't realize one could use Nz in the fashion you do here. Third, I will try QueryDefs at some point, but I am uncertain how useful they will be with SQLite down the road. – N.Barrett Dec 30 '19 at 03:59
  • I need to clarify some assumptions I'm making. I want this database to be usable throughout my company without the need for a server, and the path to the database may change from computer to computer. Knowing that path would allow pictures to be included in each entry in the main table. I may be overthinking this at the cost of usability. – N.Barrett Dec 30 '19 at 04:02
  • Is the CreateQueryDefs code the parameterized SQL statement? That's pretty advanced for me. – N.Barrett Dec 30 '19 at 09:17
  • 1
    You're welcome. Using `CreateQueryDef` to provide a means of manipulating a temporary query is but one way to work with parameterised queries - [there are many other methods](https://stackoverflow.com/a/49509616/7531598) - any of which should be favoured over concatenating values directly within a SQL statement. Your other questions would be better answered as a new question, though note that asking for the best way to do something will likely attract opinion-based answers and may therefore be closed. – Lee Mac Dec 30 '19 at 11:34
  • Parameterised queries feature a whole lot of subtlety to study in Access. I'll have to figure it out as I go along, referring back to that along the way. Until I move on to more sophisticated databases, my current "ugly" method will have to do. – N.Barrett Dec 30 '19 at 11:55
  • 1
    Don't fret - they're not as difficult as they may initially appear - in fact, because they handle escaping data types automatically (so you don't have to remember to surround values with single/double quotes, and escape quotes within the values), they are actually easier in that regard. FWIW, note that the alternative is not frowned upon for being "ugly", but rather that it leaves apps open to SQL injection attacks. Nevertheless, if my answer sufficiently answered your question, please mark it as the solution so that the question shows as resolved for others browsing the site. Thanks! – Lee Mac Dec 30 '19 at 16:38
  • Final thoughts here: 1) Your use of CreateQueryDefs syntax is new to me. Where do I find out more? 2) The link you gave states that this result can be accomplished through referring to form controls. I've done that in SQL statements elsewhere. – N.Barrett Dec 31 '19 at 04:26
  • 1) You can find out more about the CreateQueryDef method [here](https://learn.microsoft.com/en-us/office/client-developer/access/desktop-database-reference/database-createquerydef-method-dao) - the 'Remarks' describe my use of this method with a zero-length string as the query name. 2) Referencing form controls is certainly a viable alternative - though this requires creating a separate form object and so the code isn't quite as self-contained. – Lee Mac Dec 31 '19 at 14:05
1

I don't see need for nested While. Consider:

Dim booResult As Boolean
...
While booResult = False
    If fd.Show = True Then 'folder path was selected
        booResult = True
        sFolder = fd.SelectedItems(1)
    End If
Wend
June7
  • 19,874
  • 8
  • 24
  • 34