Code works, but would like advice on how to do it better 3

[DavidDrotts]

Hello,

I have built a database, Weekly Activity Report (WAR), that keeps track of the workflow in the following five work centers: DPC, DPE, DPF, DPM, and MOF. Updates to the progress of the tasks assigned to each work center must be updated weekly. In order to keep track of which work center flight chief (FC) has updated their tasks for the given week, I have created an Admin form that shows if each of the FCs have submitted their data for the given week. On this form there is a reminder button which, once clicked, will email the FCs that has tasks in the system that have not been updated. I have created the following unbound/bound controls to ensure that only a FC that has a task and has not updated for the week receives an email.

1. I use a textbox, one for each work center, with a Dcount in the control source, named DPCcount etc.. ), once a reminder button has been clicked, will email each FC to let them know that they have not updated their tasks.
2. I use a yes/no checkbox, for each FC, to keep track if the weekly update was complete.
Also, I need to ensure that there is an email address correctly inputted into the WAR so that a email reminder can be sent.

Long story short…I need to determine three conditions. 1 Have you updated for the week? 2. Do you have a task? 3. Is their an email address available to send the reminder to? If all three are true, then let the emails fly!

I struggled (I am very inexperienced) with the right code to use to make this happen and came up with the following mess, which works fine. In the interest of becoming better at this, would someone offer their two cents on how I could improve on this? Or is there a different way altogether that you can recommend for my future use. I played with nested If then statements and couldn’t get it to work.

Thank you all very much for your time, I sincerely appreciate it!

Cheers!
David

Private Sub Reminder_Click()
On Error GoTo Err_Reminder_Click
Dim rst As DAO.Recordset
Dim DPCNumber As Integer
Dim DPENumber As Integer
Dim DPFNumber As Integer
Dim DPMNumber As Integer
Dim MOFNumber As Integer

Set rst = CurrentDb().OpenRecordset("SELECT AdminLink.Link FROM AdminLink;")

If Me.DPCCount > 0 And Me.DPC = True Then
DPCNumber = 1
End If
If Me.DPECount > 0 And Me.DPE = True Then
DPENumber = 1
End If
If Me.DPFCount > 0 And Me.DPF = True Then
DPFNumber = 1
End If
If Me.DPMcount > 0 And Me.DPM = True Then
DPMNumber = 1
End If
If Me.MOFCount > 0 And Me.MOF = True Then
MOFNumber = 1
End If
'DPC Section
Select Case DPCNumber
Case 1
Debug.Print "selected 1"
If IsNull(Me.DPCFCAddress.Column(0, 0)) Then
MsgBox "There is no email address. Ensure that the Flight Chief's e-mail address has been correctly inputed.", vbOKOnly + vbInformation, _
"Weekly Activity Report"
Else
DoCmd.SendObject , "", "", Me.DPCFCAddress.Column(0, 0), Me.DPCFCAddress.Column(0, 1), "", "Please update the WAR", "An update is still required for the week of " & Me.FirstDaY & " thru " & Me.LastDay & "." & " The WAR program can be accessed by clicking on the following link (If prompted, select the 'OPEN IT' option)... " & "" & rst!link, True, ""
End If
End Select

'DPE Section
Select Case DPENumber
Case 1
Debug.Print "selected 1"
If IsNull(Me.DPEFCAddress.Column(0, 0)) Then
MsgBox "There is no email address. Ensure that the Flight Chief's e-mail address has been correctly inputed.", vbOKOnly + vbInformation, _
"Weekly Activity Report"
Else
DoCmd.SendObject , "", "", Me.DPEFCAddress.Column(0, 0), Me.DPEFCAddress.Column(0, 1), "", "Please update the WAR", "An update is still required for the week of " & Me.FirstDaY & " thru " & Me.LastDay & "." & " The WAR program can be accessed by clicking on the following link (If prompted, select the 'OPEN IT' option)... " & "" & rst!link, True, ""
End If
End Select

'DPF Section
Select Case DPFNumber
Case 1
Debug.Print "selected 1"
If IsNull(Me.DPFFCAddress.Column(0, 0)) Then
MsgBox "There is no email address. Ensure that the Flight Chief's e-mail address has been correctly inputed.", vbOKOnly + vbInformation, _
"Weekly Activity Report"
Else
DoCmd.SendObject , "", "", Me.DPFFCAddress.Column(0, 0), Me.DPFFCAddress.Column(0, 1), "", "Please update the WAR", "An update is still required for the week of " & Me.FirstDaY & " thru " & Me.LastDay & "." & " The WAR program can be accessed by clicking on the following link (If prompted, select the 'OPEN IT' option)... " & "" & rst!link, True, ""

End If
End Select

'DPM Section
Select Case DPMNumber
Case 1
Debug.Print "selected 1"
If IsNull(Me.DPMFCAddress.Column(0, 0)) Then
MsgBox "There is no email address. Ensure that the Flight Chief's e-mail address has been correctly inputed.", vbOKOnly + vbInformation, _
"Weekly Activity Report"
Else
DoCmd.SendObject , "", "", Me.DPMFCAddress.Column(0, 0), Me.DPMFCAddress.Column(0, 1), "", "Please update the WAR", "An update is still required for the week of " & Me.FirstDaY & " thru " & Me.LastDay & "." & " The WAR program can be accessed by clicking on the following link (If prompted, select the 'OPEN IT' option)... " & "" & rst!link, True, ""
End If
End Select

'MOF Section
Select Case MOFNumber
Case 1
Debug.Print "selected 1"
If IsNull(Me.MOFFCAddress.Column(0, 0)) Then
MsgBox "There is no email address. Ensure that the Flight Chief's e-mail address has been correctly inputed.", vbOKOnly + vbInformation, _
"Weekly Activity Report"
Else
DoCmd.SendObject , "", "", Me.MOFFCAddress.Column(0, 0), Me.MOFFCAddress.Column(0, 1), "", "Please update the WAR", "An update is still required for the week of " & Me.FirstDaY & " thru " & Me.LastDay & "." & " The WAR program can be accessed by clicking on the following link (If prompted, select the 'OPEN IT' option)... " & "" & rst!link, True, ""
End If
End Select





Exit_Reminder_Click:
Exit Sub

Err_Reminder_Click:
MsgBox "Your reminder has not been sent.", vbOKOnly + vbCritical, _
"Weekly Activity Report"
Resume Next
End Sub

 

[Crowley16]

I don't know if you think this would be better or not, but you could create a temporary table bound the the admin form, and store all the values, incl. e-mail addresses/section work done/admins and stuff.

then in the code, all you need to do would be open the table with a recordset, and step through each record with a for loop and send the e-mails within the loop.

you can either use the query to only pull up records which need e-mails or you can pull all the records, and use code to test if they need e-mails before sending...

it'll save you having to hard code each instance of each category...

--------------------
Procrastinate Now!

 

[DavidDrotts]

Crowley16,

Thanks for the response! A temporary table wouldn’t work because I need to keep track of the tasks per week. I should have further explained in my post that the Admin form is bound to a table that keeps track of the weekly inputs. For example this shows just the DPC section, there is also three more columns for each of the other sections (ie. DPE, DPESign, DPETime etc)
ID FirstDaY LastDay DPC DPCSign DPCTime
1 29-Aug-05 11-Sep-05 FALSE John Smith 10-sep-05
2 12-Sep-05 19-Sep-05 FALSE

This helps keep track of when and who updated for each week.

Could I still use your idea with this table even if it’s not temporary?

Thanks for your time!

Cheers!

David

 

[Crowley16]

yes

infact, you've just saved yourself work cos you don't have to make a temporary table...

--------------------
Procrastinate Now!

 

[rubbernilly]

Check via SQL if the entry exists.

If you have a table called "Chiefs" where you keep the addresses for each chief, and a table called "WeeklyUpdates" where you house their updates. Set a recordset object equal to the chiefs and step through it.

dim rs as DAO.Recordset, rsWU as dao.recordset
dim a as integer

set rs = currentdb.openRecordset("SELECT * FROM Chiefs")

rs.movelast
rs.movefirst

for a = 1 to rs.recordcount
   set rswu = currentdb.openrecordset("SELECT TOP 1 * FROM WeeklyUpdates WHERE ChiefID = '" & rs!ChiefID & "' ORDER BY UpdateDate"
   if rswu.eof true then
      'code here to send email
   else
      rswu.movefirst
      if rswu!UpdateDate < dteYourCheckDate then
         'code here to send email
   end if
Next a

set rs = nothing
set rswu = nothing

The beauty of it is you have all of the information to send your email between those two recordsets, so you can reference the Chief's email (for instance) through

rs!ChiefEmailFieldName

HTH

BTW, you will need a Reference to the DAO library in your code to do this.

 

[BSman]

I'm not sure how your data is set up, but I would do something like this:

1. Create a (grouping) query that counts by FC the numbers (by week) for open tasks, tasks updated, (etc.).

2. You can then use the query as the basis for a recordset which only includes the FCs that haven't updated and loop through each of those FCs to send the email.

It seems redundant to check the email addresses every time you look them up, but you certainly can do that.

Bob S.

 

[DavidDrotts]

Thank you all very much for your inputs. I think it’s interesting that they led me to the one area that I have cringed at learning…using SQL statements (besides the real basic stuff)in my code. When I do projects I find creative ways to avoid using SQL statements. I don’t have a good reason for doing this other then I seem to always mess it up when I try. What I have learned from your words of wisdom is that I need to get off my duff and programmatically integrate SQL statements into my projects.

Side note…I love tek-tips! I don’t know where I have learned more from…my Access books or this web site!
Thanks Again!
David

 

[BSman]

The query builder/wizard is a great tool to use to write SQL statements. Almost all statements can be written effectively by using it, as long as you know some of the "tricks", for example:

1. To create a grouping query, right click in the sort by/order by row in the grid and select (left click) "Totals". A new row will be inserted in the grid that will be for the grouping. The default says "Group by", but you can change it as needed to, for example, expression, count, sum.

2. If you need a computed type of column or a new name for a column (or for a count or sum), enter the name you want followed by a colon before the real column name (or the expression). For example, if you are using a grouping query and calculating the sum of a column called Cost, you could write: TotalCost: Cost. Instead of returning a name of SumOfCost, the result would be called TotalCost.

3. Don't be afraid to create queries in steps. Sometimes what is needed is extremely difficult to create in a single query, but creating one or more queries that will perform interim calculations, groupings, etc., and then using those queries to achieve your desired result will work extremely well.

Hope these help.

Bob S.

 

[DavidDrotts]

BSman,

Thanks for your reply. In an effort to get a handle on this I am forcing myself to create/execute all of my queries in VBA. I am working on a project that requires the user to input data and assign a user ID so that they can retrieve the data at a later time. To make this even more difficult, the ID they create can call up more then one recordset. When they are prompted to create an ID I need to warn them in the event they try to use the ID someone else has already chosen. Thus far I have built a query using the wizard that returns a record if a duplicated ID was used.
SELECT Kids.idnumber, Kids.last, Kids.First
FROM Kids
WHERE (((Kids.idnumber) In (SELECT [idnumber] FROM [Kids] As Tmp GROUP BY [idnumber] HAVING Count(*)>1 )) AND ((Kids.last) Not Like [forms]![AddKid]![Last]))
ORDER BY Kids.idnumber;

When I try to introduce it into my code I get the following error “No Value given for one or more required parameters”, on this line:
rst1.Open strSQL1, CurrentProject.Connection, adOpenKeyset, adLockOptimistic

This is my code that executes when the cursor moves from the IDnumber field on the form.

Private Sub idnumber_Exit(Cancel As Integer)
Dim rst1 As ADODB.Recordset
Dim strSQL1 As String
strSQL1 = "SELECT idnumber, last, First FROM Kids WHERE idnumber In (SELECT idnumber FROM Kids As Tmp GROUP BY idnumber HAVING Count(*)>1 ) AND last Not Like " & [Forms]![AddKid]![last] & " ORDER BY idnumber;"

Set rst1 = New ADODB.Recordset

rst1.Open strSQL1, CurrentProject.Connection, adOpenKeyset, adLockOptimistic

'If IsNull(rst1!last) Then

'Else
MsgBox "ID Number: " & rst!idnumber & " has already been selected. Please select another number.", vbOKOnly + vbCritical, _
"The Little School House"

'End If
rst1.Close
Set rst1 = Nothing


End Sub

Thanks Again!
David

 

[PHV]

First and last are reserved SQL keywords, therefore using quare brackets like this:
strSQL1 = "SELECT idnumber, [last], [First] ...

Hope This Helps, PH.
Want to get great answers to your Tek-Tips questions? Have a look at FAQ219-2884 or FAQ181-2886

 

[DavidDrotts]

Hi PHV,

Thanks for your reply!

I fixed my problem; I did not put single quotes around my variable ("'" & [Forms]![AddKid]![last] & "'"). See below…

strSQL1 = "SELECT idnumber, last, First FROM Kids WHERE idnumber In (SELECT idnumber FROM Kids As Tmp GROUP BY idnumber HAVING Count(*)>1 ) AND last Not Like " & "'" & [Forms]![AddKid]![last] & "'" & " ORDER BY idnumber;"

On a side note, thanks for the time you spend on this site. I lost count of how many posts I have read from you that applied to a problem I was currently facing.

When I grow up... I want to be just like you!

Thanks
David

 

[BSman]

Actually, you're doing it the hard way. On the AfterUpdate property of the control (I'm calling it txtNewIDNumber) where the user enters the (proposed) new ID, put code using the DCount function. Something like this (which could be shortened even more, but I'm trying to make it a little clearer here):

Dim intCount as integer
intCount = DCount("[idnumber]","[Kids]","[idnumber]=" &
me.txtNewIDNumber)
If intCount > 0 then
msgbox "Sorry, that id number has already been used"
me.txtNewIDNumber = ""
me.txtNewIDNumber.setFocus
end if


The basic idea is that if the number already has been used, DCount will return the number 1, otherwise it will be zero.

Bob S.

 

[DavidDrotts]

Hi Bob!

Thank you...again! I am going to use your suggestion. I didn’t even think about using the DCount function in an If statement, but that is why I love building applications…there is always another way to do what needs to be done.

Take Care,
David

 

[BSman]

David,

Just a general suggestion. While it's nice (?) to write everything from scratch (i.e., VBA code), why not use the Access query builder to write your SQL code. Once you have set up your query so it (hopefully) works, you can then switch to the SQL view of the query builder and copy the code into your VBA code. If you need to modify the code (perhaps to include something that the query builder doesn't easily handle), you can even do that before pasting, so you can test it.

I came to Access from Rbase, which was SQL compliant and, like most software, did not have exactly the same SQL coding as Access (which doesn't exactly match other systems), but was very close. I did have an idea of what I should be able to do with SQL code, but some of it was very difficult to achieve in Access SQL. However, the query builder has helped in writing the SQL code much more quickly (and correctly) so that if there is a part that the query builder can't handle I can concentrate on solving that, rather than writing the entire statement. And if you're dealing with a number of tables and columns in your query it can be complicated to write (and easy to make a typo).

By the way, the query builder is basically the only Access "wizard" I use. I normally find the rest to be more of a hindrance.

Bob S.