Solved VBA Function (1 Viewer)

T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
I have written a function that counts my events using an SQL statement. I did this by watching various codes being written and putting it together.
I would like advice on whether I have written the code to best practice.
The code does work and gives the correct results.

Code: 
'Used for Course Report to count the total events for the entire project
Function CntAllEvents() As Integer
    
    Dim SQL As String
    Dim dbs As Database, rst As Recordset
            
        SQL = "SELECT Count(Events_tbl.EventID) AS [Event Count] " & _
        "FROM (ListModule_tbl INNER JOIN ((Course_tbl INNER JOIN Lessons_tbl ON Course_tbl.CourseID = Lessons_tbl.CourseID) " & _
        "INNER JOIN Topics_tbl ON Lessons_tbl.LessonID = Topics_tbl.LessonID) ON ListModule_tbl.ModuleID = Course_tbl.ModuleID)  " & _
        "INNER JOIN Events_tbl ON Topics_tbl.TopicID = Events_tbl.TopicID;"

    Set dbs = CurrentDb
    Set rst = dbs.OpenRecordset(SQL)
    'Skips any modules with no events
    If rst.RecordCount = 0 Then
        CntAllEvents = 0
    Else
        rst.MoveLast
 
moke123

moke123

AWF VIP
Local time
Today, 07:05
Joined
Jan 11, 2013
Messages
3,920
I would disambiguate by adding DAO.Database and DAO.recordset.

cant see all of your code for some reason.
 
T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
Teri Bridges said:
Does this Help?

'Used for Course Report to count the total events for the entire project
Function CntAllEvents() As Integer

Dim SQL As String
Dim dbs As Database, rst As Recordset

SQL = "SELECT Count(Events_tbl.EventID) AS [Event Count] " & _
"FROM (ListModule_tbl INNER JOIN ((Course_tbl INNER JOIN Lessons_tbl ON Course_tbl.CourseID = Lessons_tbl.CourseID) " & _
"INNER JOIN Topics_tbl ON Lessons_tbl.LessonID = Topics_tbl.LessonID) ON ListModule_tbl.ModuleID = Course_tbl.ModuleID) " & _
"INNER JOIN Events_tbl ON Topics_tbl.TopicID = Events_tbl.TopicID;"

Set dbs = CurrentDb
Set rst = dbs.OpenRecordset(SQL)
'Skips any modules with no events
If rst.RecordCount = 0 Then
CntAllEvents = 0
Else
rst.MoveLast
Cduration = rst.Fields(1)
End If

End Function
Click to expand...
Sorry forget that code. Below is the correct code:

Function CntAllEvents() As Integer

Dim SQL As String
Dim dbs As Database, rst As Recordset

SQL = "SELECT Count(Events_tbl.EventID) AS [Event Count] " & _
"FROM (ListModule_tbl INNER JOIN ((Course_tbl INNER JOIN Lessons_tbl ON Course_tbl.CourseID = Lessons_tbl.CourseID) " & _
"INNER JOIN Topics_tbl ON Lessons_tbl.LessonID = Topics_tbl.LessonID) ON ListModule_tbl.ModuleID = Course_tbl.ModuleID) " & _
"INNER JOIN Events_tbl ON Topics_tbl.TopicID = Events_tbl.TopicID;"

Set dbs = CurrentDb
Set rst = dbs.OpenRecordset(SQL)
'Skips any modules with no events
If rst.RecordCount = 0 Then
CntAllEvents = 0
Else
rst.MoveLast
CntAllEvents = rst.Fields(0)
End If

End Function
 
cheekybuddha

cheekybuddha

AWF VIP
Local time
Today, 12:05
Joined
Jul 21, 2014
Messages
2,280
Teri Bridges said:
'Skips any modules with no events
If rst.RecordCount = 0 Then
Click to expand...
You don't need this with the SQL you are using.

You are selecting COUNT() which will always return a value.

So you can just shorten your code to:
Code: 
' ...
Set rst = dbs.OpenRecordset(SQL)
CntAllEvents = rst.Fields(0)
rst.Close

End Function
 
theDBguy

theDBguy

I’m here to help
Staff member
Local time
Today, 04:05
Joined
Oct 29, 2018
Messages
21,473
Don't think you need rst.MoveLast. What do you get if you comment it out?
 
T

tvanstiphout

Active member
Local time
Today, 04:05
Joined
Jan 22, 2016
Messages
222
It depends on some finer points of your database design. I'm assuming Events only exist if there are Courses, Topic, and Lessons. If that is true, then why inner join with those tables?
 
E

ebs17

Well-known member
Local time
Today, 13:05
Joined
Feb 7, 2020
Messages
1,946
Teri Bridges said:
best practice
Click to expand...
@cheekybuddha already showed you that the recordset based on this query has exactly one record with exactly one field. You can take the value directly from this. If you open a recordset, you should also close it yourself.

A good horse only jumps as high as it has to. Excessive effort wastes resources.
Code: 
Set rst = dbs.OpenRecordset(SQL, dbOpenForwardOnly)
With this option the recordset is only opened for reading at once. Therefore, this recordset requires fewer resources than dbOpenDynaset, which would be the standard but needs to manage write actions in addition to reading.

There is no variability or dynamics in the query itself. Therefore, one could/should file it as a saved query to get the tiny benefit of a saved execution plan. This would also enable you to evaluate the value of the query with a simple DLookup.
 
T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
cheekybuddha said:
You don't need this with the SQL you are using.

You are selecting COUNT() which will always return a value.

So you can just shorten your code to:
Code: 
' ...
Set rst = dbs.OpenRecordset(SQL)
CntAllEvents = rst.Fields(0)
rst.Close

End Function
Click to expand...
I tried this but received the following error:
1699551140571.png
 
E

Edgar_

Active member
Local time
Today, 06:05
Joined
Jul 8, 2023
Messages
430
Hey Teri, just a quick heads up: diving into optimization too early can turn your code into a maze of code complexity, and trust me, nobody likes solving that puzzle, not even the one who built it. Save the optimization wizardry for when it's absolutely necessary. Best practices should always be judged, the same way you should judge this advice: favor the simple, judge the complex.

Just consider that.
 
moke123

moke123

AWF VIP
Local time
Today, 07:05
Joined
Jan 11, 2013
Messages
3,920
Not sure it's still relevant but looking at your screen shot I noted ListModule_tbl.Module. Module is a reserved word and should not be used as a field name.
module.JPG
 
T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
Edgar_ said:
Hey Teri, just a quick heads up: diving into optimization too early can turn your code into a maze of code complexity, and trust me, nobody likes solving that puzzle, not even the one who built it. Save the optimization wizardry for when it's absolutely necessary. Best practices should always be judged, the same way you should judge this advice: favor the simple, judge the complex.

Just consider that.
Click to expand...
Sorry Edgar. I do not understand. I am new and learning. I have bought courses on line and been watching tutorials. I am attempting to learn what is a good practice and what is not.

The code I posted was my first attempt at using SQL in VBA. Although I received the correct expected result I was just wondering if it was written well.

Sorry for any trouble.
 
The_Doc_Man

The_Doc_Man

Immoderate Moderator
Staff member
Local time
Today, 06:05
Joined
Feb 28, 2001
Messages
27,186
Teri Bridges said:
I tried this but received the following error:
View attachment 110892
Click to expand...

The way that this shows up, it appears that you have TWO functions and we can't see the declaration of the first function. But in the second function, you name it as CntAllEvents() As Integer. Your error in the highlighted line is because you used a function name on the left-hand side of an equals sign in a place where the function is not being defined. Stated another way, if you over-define a function, it has the effect of replacing that function with the later definition. VBA therefore sees (in the highlighted line) a function name being used as though it were a variable in a definition earlier than the definition of the function.
 
T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
The_Doc_Man said:
The way that this shows up, it appears that you have TWO functions and we can't see the declaration of the first function. But in the second function, you name it as CntAllEvents() As Integer. Your error in the highlighted line is because you used a function name on the left-hand side of an equals sign in a place where the function is not being defined. Stated another way, if you over-define a function, it has the effect of replacing that function with the later definition. VBA therefore sees (in the highlighted line) a function name being used as though it were a variable in a definition earlier than the definition of the function.
Click to expand...
I have three functions CountEvents, CountAllEvents, Cduration
The third function Cduration, is one I am still working on. I am trying to sum the lesson duration total to get the overall course duration.

Code: 
'Used for Course Report to count the total events at the Module Level
Function CountEvents(MyModule As Integer) As Integer
    Dim SQL As String
    Dim dbs As Database, rst As Recordset
    

'    Set dbs = OpenDatabase(CurrentProject.Path & "\Tracker4.accdb")
    'SQL Statement made in a Qurey then pasted here
    SQL = "SELECT ListModule_tbl.ModuleID, ListModule_tbl.Module, Count(Events_tbl.EventID) AS [Event Count] " & _
        "FROM (ListModule_tbl INNER JOIN ((Course_tbl INNER JOIN Lessons_tbl ON Course_tbl.CourseID = Lessons_tbl.CourseID) " & _
        "INNER JOIN Topics_tbl ON Lessons_tbl.LessonID = Topics_tbl.LessonID) ON ListModule_tbl.ModuleID = Course_tbl.ModuleID) " & _
        "INNER JOIN Events_tbl ON Topics_tbl.TopicID = Events_tbl.TopicID " & _
        "GROUP BY ListModule_tbl.ModuleID, ListModule_tbl.Module, Course_tbl.CourseID, Course_tbl.CatalogID, Lessons_tbl.LessonID " & _
        "HAVING (((ListModule_tbl.ModuleID)=" & Str(MyModule) & "));"
        
    Set dbs = CurrentDb
    Set rst = dbs.OpenRecordset(SQL)
     'Skips any modules with no event
'    If rst.RecordCount = 0 Then
'        CountEvents = 0
'    Else
'        rst.MoveLast
'        CountEvents = rst.Fields(2)
'    End If
        Set rst = dbs.OpenRecordset(SQL)
        CountEvents = rst.Fields(0)
        rst.Close

End Function

'Used for Course Report to count the total events for the entire project
Function CntAllEvents() As Integer
    
    Dim SQL As String
    Dim dbs As Database, rst As Recordset
            
        SQL = "SELECT Count(Events_tbl.EventID) AS [Event Count] " & _
                "FROM (ListModule_tbl INNER JOIN ((Course_tbl INNER JOIN Lessons_tbl ON Course_tbl.CourseID = Lessons_tbl.CourseID) " & _
                "INNER JOIN Topics_tbl ON Lessons_tbl.LessonID = Topics_tbl.LessonID) ON ListModule_tbl.ModuleID = Course_tbl.ModuleID)  " & _
                "INNER JOIN Events_tbl ON Topics_tbl.TopicID = Events_tbl.TopicID;"

        Set dbs = CurrentDb
        Set rst = dbs.OpenRecordset(SQL)
    'Skips any modules with no events
'    If rst.RecordCount = 0 Then
'        CntAllEvents = 0
'    Else
'        rst.MoveLast
'        CntAllEvents = rst.Fields(0)
'    End If
        Set rst = dbs.OpenRecordset(SQL)
        CntAllEvents = rst.Fields(0)
        rst.Close

End Function

'Used to calculate the Course Duration
Function Cduration() As Integer
    
    Dim SQL As String
    Dim dbs As Database, rst As Recordset
            
        SQL = "SELECT Course_tbl.CatalogID, Sum(Lessons_tbl.LessonDuration) AS [Course Duration] " & _
                "FROM (ListModule_tbl INNER JOIN Course_tbl ON ListModule_tbl.[ModuleID] = Course_tbl.[ModuleID]) " & _
                "INNER JOIN Lessons_tbl ON Course_tbl.[CourseID] = Lessons_tbl.[CourseID] " & _
                "GROUP BY Course_tbl.CatalogID;"

    Set dbs = CurrentDb
    Set rst = dbs.OpenRecordset(SQL)
    'Skips any courses with No lessons
'    If rst.RecordSum = 0 Then
'        Cduration = 0
'    Else
'        rst.MoveLast
'        Cduration = rst.Fields(1)
'    End If
        Set rst = dbs.OpenRecordset(SQL)
        Cduration = rst.Fields(1)
        rst.Close
End Function
 
T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
Teri Bridges said:
I have three functions CountEvents, CountAllEvents, Cduration
The third function Cduration, is one I am still working on. I am trying to sum the lesson duration total to get the overall course duration.

Code: 
'Used for Course Report to count the total events at the Module Level
Function CountEvents(MyModule As Integer) As Integer
    Dim SQL As String
    Dim dbs As Database, rst As Recordset
   

'    Set dbs = OpenDatabase(CurrentProject.Path & "\Tracker4.accdb")
    'SQL Statement made in a Qurey then pasted here
    SQL = "SELECT ListModule_tbl.ModuleID, ListModule_tbl.Module, Count(Events_tbl.EventID) AS [Event Count] " & _
        "FROM (ListModule_tbl INNER JOIN ((Course_tbl INNER JOIN Lessons_tbl ON Course_tbl.CourseID = Lessons_tbl.CourseID) " & _
        "INNER JOIN Topics_tbl ON Lessons_tbl.LessonID = Topics_tbl.LessonID) ON ListModule_tbl.ModuleID = Course_tbl.ModuleID) " & _
        "INNER JOIN Events_tbl ON Topics_tbl.TopicID = Events_tbl.TopicID " & _
        "GROUP BY ListModule_tbl.ModuleID, ListModule_tbl.Module, Course_tbl.CourseID, Course_tbl.CatalogID, Lessons_tbl.LessonID " & _
        "HAVING (((ListModule_tbl.ModuleID)=" & Str(MyModule) & "));"
       
    Set dbs = CurrentDb
    Set rst = dbs.OpenRecordset(SQL)
     'Skips any modules with no event
'    If rst.RecordCount = 0 Then
'        CountEvents = 0
'    Else
'        rst.MoveLast
'        CountEvents = rst.Fields(2)
'    End If
        Set rst = dbs.OpenRecordset(SQL)
        CountEvents = rst.Fields(0)
        rst.Close

End Function

'Used for Course Report to count the total events for the entire project
Function CntAllEvents() As Integer
   
    Dim SQL As String
    Dim dbs As Database, rst As Recordset
           
        SQL = "SELECT Count(Events_tbl.EventID) AS [Event Count] " & _
                "FROM (ListModule_tbl INNER JOIN ((Course_tbl INNER JOIN Lessons_tbl ON Course_tbl.CourseID = Lessons_tbl.CourseID) " & _
                "INNER JOIN Topics_tbl ON Lessons_tbl.LessonID = Topics_tbl.LessonID) ON ListModule_tbl.ModuleID = Course_tbl.ModuleID)  " & _
                "INNER JOIN Events_tbl ON Topics_tbl.TopicID = Events_tbl.TopicID;"

        Set dbs = CurrentDb
        Set rst = dbs.OpenRecordset(SQL)
    'Skips any modules with no events
'    If rst.RecordCount = 0 Then
'        CntAllEvents = 0
'    Else
'        rst.MoveLast
'        CntAllEvents = rst.Fields(0)
'    End If
        Set rst = dbs.OpenRecordset(SQL)
        CntAllEvents = rst.Fields(0)
        rst.Close

End Function

'Used to calculate the Course Duration
Function Cduration() As Integer
   
    Dim SQL As String
    Dim dbs As Database, rst As Recordset
           
        SQL = "SELECT Course_tbl.CatalogID, Sum(Lessons_tbl.LessonDuration) AS [Course Duration] " & _
                "FROM (ListModule_tbl INNER JOIN Course_tbl ON ListModule_tbl.[ModuleID] = Course_tbl.[ModuleID]) " & _
                "INNER JOIN Lessons_tbl ON Course_tbl.[CourseID] = Lessons_tbl.[CourseID] " & _
                "GROUP BY Course_tbl.CatalogID;"

    Set dbs = CurrentDb
    Set rst = dbs.OpenRecordset(SQL)
    'Skips any courses with No lessons
'    If rst.RecordSum = 0 Then
'        Cduration = 0
'    Else
'        rst.MoveLast
'        Cduration = rst.Fields(1)
'    End If
        Set rst = dbs.OpenRecordset(SQL)
        Cduration = rst.Fields(1)
        rst.Close
End Function
Click to expand...
1699552478494.png
 
E

Edgar_

Active member
Local time
Today, 06:05
Joined
Jul 8, 2023
Messages
430
Teri Bridges said:
Sorry Edgar. I do not understand. I am new and learning. I have bought courses on line and been watching tutorials. I am attempting to learn what is a good practice and what is not.

The code I posted was my first attempt at using SQL in VBA. Although I received the correct expected result I was just wondering if it was written well.

Sorry for any trouble.
Click to expand...
I apologize for confusing you. We all learn differently, what I'm trying to tell you is to stick with the simplest version of some code until you see that it's necessary to make it more complex. I'm also trying to tell you that it's OK to have code that just works, as long as you understand it. It's just a little tip you'd probably be happy with, because it's easy to be overwhelmed by "best practices" that involve learning things you don't necessarily need or add very little.
 
T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
Edgar_ said:
I apologize for confusing you. We all learn differently, what I'm trying to tell you is to stick with the simplest version of some code until you see that it's necessary to make it more complex. I'm also trying to tell you that it's OK to have code that just works, as long as you understand it. It's just a little tip you'd probably be happy with, because it's easy to be overwhelmed by "best practices" that involve learning things you don't necessarily need or add very little.
Click to expand...
Now That I understand. Thank you. LOL :)
 
cheekybuddha

cheekybuddha

AWF VIP
Local time
Today, 12:05
Joined
Jul 21, 2014
Messages
2,280
You have a different SQL query in the picture posted in Post #15, and you are selecting a lot more fields.

What do you want from this function (CountEvents()) ?

Is there an error? (Why is the line highlighted?)
 
T

Teri Bridges

Member
Local time
Today, 06:05
Joined
Feb 21, 2022
Messages
186
cheekybuddha said:
You have a different SQL query in the picture posted in Post #15, and you are selecting a lot more fields.

What do you want from this function (CountEvents()) ?

Is there an error? (Why is the line highlighted?)
Click to expand...
Yes, I had only shown the first function in the original post. It is highlighted because it failed and when I clicked debug, it highlighted that row. This function counts all the events by Module.
It is working as I had it written. I don't want to confuse things and be a bother. Being new to coding I wanted to check and see if I had any rookie mistakes that should not be done when coding.
 
cheekybuddha

cheekybuddha

AWF VIP
Local time
Today, 12:05
Joined
Jul 21, 2014
Messages
2,280
Please can you add a Debug.Print SQL line after setting the SQL, and post the output from the Immediate Window (Ctrl+G) here after you try and run the function.

Also, you can just try changing:
Code: 
' ...
    CountEvents = rst.Fields(0)
' ...
to:
Code: 
' ...
    CountEvents = rst.Fields("Event Count")
' ...
 
You must log in or register to reply here.

Similar threads

Z
Assistance - Issue with Expression Function in SQL Query and Invoking Function in VBA Editor
Replies
4
Views
444
June7
June7
F
Add query data to existing export function
Replies
1
Views
356
MarkK
MarkK
A
Function produces Error 3027 - Cannot update when running rs.edit
Replies
19
Views
1,287
Pat Hartman
P
F
Solved Add result from one function to another export function
Replies
18
Views
709
FoolzRailer
F
C
Solved Using a control on a form to display the most frequently occurring value in a field
Replies
8
Views
863
Pat Hartman
P

Users who are viewing this thread

Total: 1 (members: 0, guests: 1)
Top Bottom