Better Code

[ljolivo]

Hi All,

I'm fairly new to vba, so as I go, I'd like to improve how I write code. I created some VBA code to read all of the records in two different tables within an Access database and return all values within a specified field in each table. It works fine, but how can I improve this code?


Function GetPanlList() As String()
Dim sqlString() As String
Dim sqlFieldName() As String
Dim sqlTableName() As String
Dim RecSetEnd As Long
Dim RecSetCnt As Long
Dim SendPanlArr() As String
Dim i As Long
ReDim sqlString(1)
ReDim sqlFieldName(1)
ReDim sqlTableName(1)

SetDBConnection ("FRB_EVR_Database.mdb") 'connects to database
Set RecSet = Nothing
sqlTableName(0) = "Panel Board"
sqlFieldName(0) = "PanelID"
sqlTableName(1) = "Sub - Panel Board"
sqlFieldName(1) = "SubPanelID"
RecSetEnd = 0
RecSetCnt = 0
i = 0
CadInputQueue.SendCommand "choose none" 'application specific
CommandState.StartDefaultCommand 'application specific
For i = 0 To 1
sqlString(i) = "SELECT DISTINCT " & sqlFieldName(i) & " FROM [" & sqlTableName(i) & "] WHERE " & sqlFieldName(i) & " Is Not Null ORDER BY " & sqlFieldName(i) & ";"
RecSet.Open sqlString(i), myDB, adOpenStatic, adLockOptimistic


'get and set upper limit of array
RecSet.MoveLast
RecSetEnd = RecSetEnd + RecSet.RecordCount
RecSet.MoveFirst
ReDim Preserve SendPanlArr(RecSetEnd)

'Create list from Panel and Sub - Panel Tables
RecSet.MoveFirst
Do
SendPanlArr(RecSetCnt) = RecSet.Fields(sqlFieldName(i))
RecSet.MoveNext
RecSetCnt = RecSetCnt + 1
Loop Until RecSet.EOF
RecSet.Close
Next i
myDB.Close 'public constant declared elsewhere
Set RecSet = Nothing
Set myDB = Nothing
GetPanlList = QuickSort(SendPanlArr, LBound(SendPanlArr), UBound(SendPanlArr)) 'sort the array and end function
End Function

[jonh]

Hello.

Not bad! As you are learning do whatever you like and have fun! :)

A couple of things stand out.

Rule #1 is don't write code if you don't need to (learn what features the application already offers.)
Especially don't write code if you could use a query instead.
Queries are highly optimised and superfast whereas using VBA to read or update recordsets is super sloooow by comparison.

In this case I think you could use a union query...


select x from table1 where x = 'y'
union
select x from table2 where x = 'y'
order by x

Why are you adding the data to arrays? What is the output?

You have also hard coded data, which means you need to change the code if the data changes.

quick example :


Private Sub Command0_Click()
Dim rs As DAO.Recordset
Set rs = UnionQry(Me!Combo1)
If Not rs Is Nothing Then
Debug.Print "Field: " & rs(0).Name
Do Until rs.EOF
Debug.Print rs(0)
rs.MoveNext
Loop
End If
End Sub

Public Function UnionQry(id As Long) As DAO.Recordset
Dim rs As DAO.Recordset
Dim s As String, sql As String
Set rs = CurrentDb.OpenRecordset("select tablename, fieldname from tquery where qryID=" & id)
Do Until rs.EOF
s = "select [%] from [@] where [%] is not null"
s = Replace(s, "%", rs(1))
s = Replace(s, "@", rs(0))
If Len(sql) Then sql = sql & vbNewLine & "union" & vbNewLine
sql = sql & s
rs.MoveNext
Loop
'Debug.Print sql
If Len(sql) Then Set UnionQry = CurrentDb.OpenRecordset(sql)
End Function

[ljolivo]

Thank you, jonh! Hadn't learned the Union option yet. couple of questions on your UnionQry function:
What is "tquery"?
What is "id"?

the query results are going to go into a userform combobox. I actually end up using the same list in at least one other combobox, and I'm trying to avoid having to query the db multiple times for the same information. If I can simply create a semi-permanent list from a single query, then it runs faster.

I have the fields and tables hardcoded because they won't change once I implement the program. The record data will, but the fields and tables won't. It is obviously very static this way, but i don't really want the user to be able to go in and choose.

[HiTechCoach]

I'm trying to avoid having to query the db multiple times for the same information.

Why are you trying to avoid this?

If this is a single user database, the back end (split database) is on a slow lan, and the list will rarely every change then avoiding having to query the db multiple times for the same information makes sense.

If you are wanting to learn to create multiple concurrent user databases then you you will need to rethink the idea that "having to query the db multiple times" is bad.

If you have properly normalized the table design, then querying the db multiple times can be very efficient.

With Acess, using stores queries is generall better. Acess will optixes the query before it is stored. When you execute a query created where the SQL is created with VBA then it must be optimized each time before it is executed.


Note: The Access databse engines (JEt for ,.mdb and ACE for .accdb) use DAO internally. When you write ADO code, it must be translated back to DAO before it is executed at the database ending level. I would urge you to learn DAO if you will continue to would with Access tables and recordsets.

[jonh]

tquery doesn't matter if you're set on hardcoding the values, but since you asked

It's a new table to hold the query values (table/field names).
ID is used to select which query values get inserted into the sql.

tquery table
QryID TableName FieldName
1 table1 x
1 table2 x
2 table1 x
2 table2 y
2 table3 z

result sql for ID 1:
select x from table1 union
select x from table2

result sql for ID 2:
select x from table1 union
select y from table2 union
select z from table3

[ljolivo]

Thanks, Boyd. I'm pretty sure I haven't properly normalized the table design as you mentioned. I'm almost as new to Access as I am to VBA, so there's a LOT I don't know about it. I've basically created a number of tables with various fields, and populated SOME of the fields in the recordsets. I do have multiple users, but the database could be on a slow lan. We often work remotely. The list won't change daily, but it may change monthly. I'll have to look into stored queries for sure. Right now I'm using JET for the engine, because I don't think my application supports DAO connectivity. At least, I haven't been able to find how to do so if it does.

[ljolivo]

Thanks, jonh.

[HiTechCoach]

Right now I'm using JET for the engine, because I don't think my application supports DAO connectivity. At least, I haven't been able to find how to do so if it does.

JET uses DAO. If you are writing VBA code in Access then DAO being used. Access/JET/ACE uses DAO internally for all table operations.

If yu are wanting to learn to develop applications with Access then I would urge you to rethink what you are attempting. It really is not the way Access work best. Form what I have seen in the code you posting will probably never be used in an Access with VBA application . I have create over 1000 Access applications. I have a full-blown Accounting system create in Access.

If you really want to write a lot of code to do simple things that Access can do for you then Access is probably not the development tool you should be using.

If you want to use Access as a development tool then I would highly recommend that you first learn how Access really works. There is already a lot it can already do with all the built in functionality taht does not require so much VBA code.. You will probably be very surprised to find out that you need very little VBA code to create very powerful applications.

[ljolivo]

Thanks, Boyd. I'm not actually writing VBA IN Access at this point. I'm writing it in a cadd program which needs to access an Access database. JET may still use DAO from this direction, but I just don't know if the cadd application will support it. Might, though. still looking into it. Wherever possible I will definitely want to leverage the best tool for the job, though, so thanks for your help. Eventually, I'll want to do other things with the data in Access, so i'll need to learn it eventually.

[HiTechCoach]

Interesting .... which cadd program are you using that supports VBA code?

Did you now that every application that support VBA does not have the same VBA. Access VBA is not the same as Word or Excel VBA. Your cadd program's VBA is not the same as the VBA used in Access. That is why it is call Viscaul Basich for Applications where each Application has its own flavor customized to the applications object model.

To be accurate, yu are writing VBA in your cadd programs version of VBA to access a JET (.mdb) database. This is NOT Access VBA. To Write Access VBA you must be running the Microsoft Access program


FYI: When connecting to an access database using an ODBC drive you do have to use ADO.

[jonh]

Just to clarify some of the above.

I think people forget that MSAccess is NOT a database.
Data is stored and accessed by the database engine which is nothing to do with MSAccess.
MSAccess is just a GUI for easily setting up tables and building user interfaces.

If you want to build your user interface outside of Access (in a CAD program or whatever), then as long as the/a database engine is installed and you can create a reference to it you do not need to use MSAccess AT ALL. EVER.

The VBA language doesn't change between applications. VBA is VBA. You might have different VERSIONS of VBA installed, i.e. if you have an old version of Word and new version of Access, but in general all Office apps should use the same VBA dll.
C:\Program Files\Common Files\microsoft shared\VBA\VBA[version#]

The object model does change because, obviously, Access, Word, Excel, etc are completely different applications. Excel might use tables, but they will be completely different things to Access tables.
Go into a module and press F2 to show the object browser.
In the top left you can choose which library to view (Access Object Library, DAO, ADODB, stdole, VBA, etc.)
If you are in Word and do the same thing, you will see Word Object Library, not Access Object Library (unless you have added a reference to Access for some reason).

If you select the VBA library you get a list of available classes. Select Math to see avalable members of the Math object.
You might use ABS(number) in your code but VBA is clever enough to know that you actually meant VBA.Math.ABS(number).

So, what you can access through VBA and how you access it is defined by the library dll's that you have referenced, not by the application.


As for DAO and ADO. Microsoft have been trying to phase out DAO for years.
They switched the default for Access VBA from DAO to ADO before changing it back again. I guess because people find it easier to use.
If you are using ADO without problems stick with it.

[ljolivo]

Thanks, gentlemen.
I'm using Microstation as my cadd program. We're essentially linking drawing elements to records in a database which then contain a bunch of information about the elements such as manufacturer, model, size, etc. I'm using Access myself to set up the database and be able to see whether or not my program is doing what I think it is.

I think have the Access Object Library loaded (it just says "Access" in my Object Browser), so now my question is, would it be better to use the Access objects to work with the data in the database or the regular VBA objects? My editing environment as well as my input environment will be from CADD. I don't intend (at this point) for most of my users to have the ability to open Access and work in the database directly. I may want to do that myself later in the process, but not at the moment.

[jonh]

I don't see why you need Access referenced in the CAD app.

I would add a reference to "Microsoft ActiveX Data Objects x.x Library"

And use code like this (which you should be able to test in Access and Microstation.)


Dim dbConn As New ADODB.Connection
Dim rs As New ADODB.Recordset

'open a connection to the database
dbConn.Open "Provider=Microsoft.ACE.OLEDB.12.0;User ID=Admin;Data Source=C:\test\test.mdb;"

'get the data
rs.Open "select x from table1 union select x from table2", dbConn

Do Until rs.EOF
'do something with the data
Debug.Print rs(0).Value
rs.MoveNext
Loop
rs.Close
dbConn.Close

[ljolivo]

Thanks, Jonh. You're right. I don't actually need to open Access to create/read/write the database. I posted in this forum because it was the closest match for what I'm actually doing. I actually use the JET version of that connection string from CAD to the DB in the connection module, but otherwise, my process is essentially what you have written above. Through all of this, I actually see how I could hard-code less, though, which is what I'm looking for. Thanks, guys.

[HiTechCoach]

I think have the Access Object Library loaded (it just says "Access" in my Object Browser), so now my question is, would it be better to use the Access objects to work with the data in the database or the regular VBA objects?

If you only are reading and writing to the table in the JET (.mdb) database then the Access object model is not being used. Access objects that require Access to be installed are the forms and , reports.

If you are writing all the VBA code in the cadd app then Access should not be required (installed).



I don't intend (at this point) for most of my users to have the ability to open Access and work in the database directly. I may want to do that myself later in the process, but not at the moment.

When your cadd VBA connects the the MDb you are working directly in the database (tables).

VB, ASP, .NET, etc can all work directly with the tables in an .MDB or .ACCDB database file.on a machines that does NOT have the Access application installed.

The other Access objects like forms, reports, macros, and code modules do require the actual Access application to be installed to execute (run) them.