2
\$\begingroup\$

I wrote this very "general use" function, because I needed that in some reporting. I was wondering if there is any suggetsion for improvement.

Function sql2List(sSql As String, Optional quote As String, Optional separ As String = ", ") As String
    Static db As DAO.Database
    Dim rs As DAO.Recordset
    Dim result As String
    'Debug.Print Now, sSql
    If db Is Nothing Then Set db = CurrentDb
    Set rs = db.OpenRecordset(sSql, dbOpenSnapshot)
    Do Until rs.EOF
        result = result & quote & rs.Fields(0).Value & quote & separ
        rs.MoveNext
    Loop
    rs.Close
    Set rs = Nothing
    'Set db = Nothing
    If Len(result) > Len(separ) Then sql2List = Left(result, Len(result) - Len(separ))
End Function

I call it from control or module or other queries like this:

strList = sql2List("select dt from planning where client = " & clientId)
\$\endgroup\$
1
\$\begingroup\$

In the context of the function

Function sql2List(sSql As String, Optional quote As String, Optional separ As String = ", ") As String
Static db As DAO.Database
Dim rs As DAO.Recordset
Dim result As String
'Debug.Print Now, sSql
If db Is Nothing Then Set db = CurrentDb
Set rs = db.OpenRecordset(sSql, dbOpenSnapshot)
Do Until rs.EOF
    result = result & quote & rs.Fields(0).Value & quote & separ
    rs.MoveNext
Loop
rs.Close
Set rs = Nothing
'Set db = Nothing
If Len(result) > Len(separ) Then sql2List = Left(result, Len(result) - Len(separ))
End Function

The static declaration of db is probably not a good idea.

This (my reformatting is just my preference) is probably better

Set db = CurrentDb
strList = sql2List(db, "select dt from planning where client = " & clientId)
Set db = Nothing

Function sql2List(db as DAO.Database,sSql As String, Optional quote As String, Optional separ As String = ", ") As String
Dim rs As DAO.Recordset
Dim result As String
Dim RetValue as string
  RetValue = ""
  'Debug.Print Now, sSql
  If not(db Is Nothing) = True Then
     Set rs = db.OpenRecordset(sSql, dbOpenSnapshot)
     Do Until rs.EOF
         result = result & quote & rs.Fields(0).Value & quote & separ
         rs.MoveNext
     Loop
     rs.Close
     Set rs = Nothing
     If Len(result) > Len(separ) Then
        RetValue = Left(result, Len(result) - Len(separ))
     endif
  ELSE
     RetValue =""
  endif
  sql2List= Trim(RetValue) ' I tend to want to remove leading and training spaces
End Function

There was nothing (much) wrong with what you did other that the static inside the function as that made the function less utilitarian because there was no way that function could ever be used for any connection other than the first one. It just so happens to work because you do not desire a different connection.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ AFAIK, making the db a parameter would prevent calling that function from within a control or a query, which is whre I use it the most. Other wise I could re-Set the db as current db every call, but I thought the Stitic trick would make me save a few milliseconds. \$\endgroup\$ – Patrick Honorez Apr 27 '17 at 16:47
  • \$\begingroup\$ Well there is the Function and there is the Caller. I do not know of any reason your Control / query can not invoke a separate Sub / Function that then implements the non-function code. The first three lines of code I have separated from the Function itself. Ultimately all I was doing was making the Function a true utility function ... I accept that may not be within the scope of what you want to do. If so my mistake. So you can leave it as is and if I were grading things I would not ding you :) \$\endgroup\$ – Enigma Maitreya Apr 27 '17 at 17:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.