HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Notify user of upcoming expiration of subscription

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
notifyexpirationuserupcomingsubscription

Problem

Here is some VB code that is part of a major system of apps that I have been asked to maintain, one of the more fun things that I get to do is rewrite some code here and there, I have done a little bit of that here, I finally have it functioning properly and would like to see if I am still on an upward stride in my VB learning.

```
Module Module1

Private Function SQL_Connection_String() As String
Dim Connection As String
#If DEBUG Then
Connection = {TestConnectionString}
#ElseIf CONFIG = "Release" Then
Connection = {ProductionConnectionString}
#End If
Return Connection
End Function
Private Function test_Email() As String
Return {TestersEmail}
End Function

Private sLetterText As String

Sub Main()
Dim sSource As String
Dim sLog As String
Dim sEvent As String
Dim sMachine As String

sSource = "OLMJSubscriberNotification"
sLog = "Application"
sEvent = "Error"
sMachine = "."

If Not EventLog.SourceExists(sSource) Then
EventLog.CreateEventSource(sSource, sLog)
End If

Dim ErrorLog As New EventLog(sLog, sMachine, sSource)
Dim SentLog As New EventLog(sLog, sMachine, sSource)

ReadSQL(SentLog, ErrorLog)
End Sub

Public Sub ReadSQL(ByRef ErrorLog As EventLog, ByRef SentLog As EventLog)
Dim sText As New String("")
Dim sRecip As New String("")
Dim mailSent As Integer
Dim dr As SqlDataReader
Dim sRecipList As New String("")

Dim cnn As New SqlConnection(SQL_Connection_String())

Const SQLstr = "SELECT UserName, SubscriptionEnd, Email,UserID, AgencyName FROM Users " & _
"WHERE(DateDiff(Day, GETDATE(), SubscriptionEnd) = 0 " & _
"AND (DATEDIFF(day,GETDATE(), NotificationSent) 0 Then
sRecipList = "The following " & mailSent.ToString & " subscribers received expiration mail " & Now.ToString & ":"

Solution

This query uses DATEDIFF() inefficiently:

Const SQLstr = "SELECT UserName, SubscriptionEnd, Email,UserID, AgencyName FROM Users " & _
    "WHERE(DateDiff(Day, GETDATE(), SubscriptionEnd) = 0 " & _
    "AND (DATEDIFF(day,GETDATE(), NotificationSent) < -10 OR NotificationSent IS NULL)"


To accomplish that, the server would have to run DATEDIFF() on every single row in the Users table. Instead, you should calculate the cutoff dates just once:

SELECT UserName, SubscriptionEnd, Email,UserID, AgencyName
    FROM Users
    WHERE
        SubscriptionEnd = CAST(GETDATE() AS DATE)
        AND (NotificationSent IS NULL
             OR NotificationSent < CAST(DATEADD(Day, -10, GETDATE()) AS DATE));


(The casts truncate the results of DATEADD() to midnight.)

Code Snippets

Const SQLstr = "SELECT UserName, SubscriptionEnd, Email,UserID, AgencyName FROM Users " & _
    "WHERE(DateDiff(Day, GETDATE(), SubscriptionEnd) <= 10) " & _
    "And DateDiff(Day, GETDATE(), SubscriptionEnd) >= 0 " & _
    "AND (DATEDIFF(day,GETDATE(), NotificationSent) < -10 OR NotificationSent IS NULL)"
SELECT UserName, SubscriptionEnd, Email,UserID, AgencyName
    FROM Users
    WHERE
        SubscriptionEnd <= CAST(DATEADD(Day, 10, GETDATE()) AS DATE)
        AND SubscriptionEnd >= CAST(GETDATE() AS DATE)
        AND (NotificationSent IS NULL
             OR NotificationSent < CAST(DATEADD(Day, -10, GETDATE()) AS DATE));

Context

StackExchange Code Review Q#47257, answer score: 6

Revisions (0)

No revisions yet.