Generating fiscal calendar dates

Question

I need to generate a T-SQL script for loading data into this table:

create table dwd.FiscalCalendars (
 _Id int not null identity(1,1)
 ,_DateInserted datetime not null
 ,_DateUpdated datetime null
 ,CalendarDate date not null
 ,CalendarDayOfWeek int not null
 ,CalendarDayOfMonth int not null
 ,CalendarDayOfYear int not null
 ,CalendarWeekOfYear int not null
 ,CalendarMonthOfYear int not null
 ,CalendarYear int not null
 ,FiscalDayOfWeek int not null
 ,FiscalDayOfMonth int not null
 ,FiscalDayOfQuarter int not null
 ,FiscalDayOfYear int not null
 ,FiscalWeekOfMonth int not null
 ,FiscalWeekOfQuarter int not null
 ,FiscalWeekOfYear int not null
 ,FiscalMonthOfQuarter int not null
 ,FiscalMonthOfYear int not null
 ,FiscalQuarterOfYear int not null
 ,FiscalYear int not null
 ,Holiday nvarchar(50) null
 ,constraint PK_FiscalCalendars primary key clustered (_Id asc)
 ,constraint NK_FiscalCalendars unique (CalendarDate)
);

In order to generate the script, I first need to generate each record. I decided to write some VBA code to do that. Here's the function I came up with - I'll make another to work backwards, so as to generate records for previous fiscal years for which I don't know the start date - hence I called this one "GenerateForwardCalendar":

Private Function GenerateForwardCalendar(ByVal startDate As Date, ByVal fStartYear As Integer, ByVal years As Integer) As Collection
 Dim result As New Collection
 Dim current As FiscalCalendarDate
 Dim currentDate As Date
 currentDate = startDate
 Dim fYear As Integer
 Dim fQuarterOfYear As Integer
 Dim fMonthOfQuarter As Integer
 Dim fWeekOfMonth As Integer
 Dim fDayOfWeek As Integer
 For fYear = fStartYear To fStartYear + years
 Set current = FiscalCalendarDate.Create(currentDate, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, fYear, vbNullString)
 For fQuarterOfYear = 1 To 4
 current.FiscalDayOfQuarter = 1
 current.FiscalWeekOfQuarter = 1
 current.FiscalMonthOfQuarter = 1
 For fMonthOfQuarter = 1 To 3
 current.FiscalDayOfMonth = 1
 current.FiscalWeekOfMonth = 1
 If IIf(IsLeapYear(current.calendarYear) And current.FiscalMonthOfYear = 12, True, fMonthOfQuarter Mod 2 = 0) Then
 For fWeekOfMonth = 1 To 5
 GoSub ProcessWeekDays
 Next
 Else
 For fWeekOfMonth = 1 To 4
 GoSub ProcessWeekDays
 Next
 End If
 current.FiscalMonthOfQuarter = current.FiscalMonthOfQuarter + 1
 current.FiscalMonthOfYear = current.FiscalMonthOfYear + 1
 Next
 current.FiscalQuarterOfYear = current.FiscalQuarterOfYear + 1
 Next
 Next
CleanExit:
 Set GenerateForwardCalendar = result
 Exit Function
ProcessWeekDays:
 For fDayOfWeek = 1 To 7
 current.FiscalDayOfWeek = fDayOfWeek
 Dim item As FiscalCalendarDate
 Set item = FiscalCalendarDate.Copy(current)
 Debug.Print item.ToString
 result.Add item
 currentDate = currentDate + 1
 current.CalendarDate = currentDate
 current.FiscalDayOfWeek = current.FiscalDayOfWeek + 1
 current.FiscalDayOfMonth = current.FiscalDayOfMonth + 1
 current.FiscalDayOfQuarter = current.FiscalDayOfQuarter + 1
 current.FiscalDayOfYear = current.FiscalDayOfYear + 1
 Next
 current.FiscalWeekOfMonth = current.FiscalWeekOfMonth + 1
 current.FiscalWeekOfQuarter = current.FiscalWeekOfQuarter + 1
 current.FiscalWeekOfYear = current.FiscalWeekOfYear + 1
 Return
End Function

Here's the IsLeapYear function I'm using, for the leap years shall have a 5th week in the last month of the 4th quarter, making them 53-week years:

Public Function IsLeapYear(ByVal calendarYear As Integer) As Boolean
 IsLeapYear = (Month(DateSerial(calendarYear, 2, 29)) = 2)
End Function

So, this gives me a collection of FiscalCalendarDate objects, which I will be iterating later, and passing to this function:

Private Function GenerateInsertStatement(ByVal value As FiscalCalendarDate) As String
 Dim result As String
 With value
 result = Framework.Strings.Format("INSERT INTO dwd.FiscalCalendars (_DateInserted,CalendarDate,CalendarDayOfWeek,CalendarDayOfMonth,CalendarDayOfYear,CalendarWeekOfMonth,CalendarWeekOfYear,CalendarMonthOfYear,CalendarYear," & _
 "FiscalDayOfWeek,FiscalDayOfMonth,FiscalDayOfQuarter,FiscalDayOfYear,FiscalWeekOfMonth,FiscalWeekOfQuarter,FiscalWeekOfYear,FiscalMonthOfQuarter,FiscalMonthOfYear,FiscalQuarterOfYear,FiscalYear,Holiday) ", _
 "VALUES (@ts,'{0}',{1},{2},{3},{4},{5},{6},{7},{8},{9},{10},{11},{12},{13},{14},{15},{16},{17},{18},'{19}');", _
 .CalendarDate, .CalendarDayOfWeek, .CalendarDayOfMonth, .CalendarDayOfYear, _
 .CalendarWeekOfMonth, .CalendarWeekOfYear, .CalendarMonthOfYear, .calendarYear, _
 .FiscalDayOfWeek, .FiscalDayOfMonth, .FiscalDayOfQuarter, .FiscalDayOfYear, _
 .FiscalWeekOfMonth, .FiscalWeekOfQuarter, .FiscalWeekOfYear, _
 .FiscalMonthOfQuarter, .FiscalMonthOfYear, .FiscalQuarterOfYear, .FiscalYear, .Holiday)
 End With
End Function

Could this code be improved? The idea isn't to execute the T-SQL, merely to generate it.

• Mathieu Guindon: Note, the "Holiday" field will be manually updated later on.

Answer 1

Why duplicate the for loop?

If ...
 For fWeekOfMonth = 1 To 5
 GoSub ProcessWeekDays
 Next
Else
 For fWeekOfMonth = 1 To 4
 GoSub ProcessWeekDays
 Next
End If

I suggest to store the 5 or 4 in a variable and parameterize one loop with it.

---

I can see this easily becoming a nightmare:

result = Framework.Strings.Format("INSERT INTO dwd.FiscalCalendars (_DateInserted,CalendarDate,CalendarDayOfWeek,CalendarDayOfMonth,CalendarDayOfYear,CalendarWeekOfMonth,CalendarWeekOfYear,CalendarMonthOfYear,CalendarYear," & _
 "FiscalDayOfWeek,FiscalDayOfMonth,FiscalDayOfQuarter,FiscalDayOfYear,FiscalWeekOfMonth,FiscalWeekOfQuarter,FiscalWeekOfYear,FiscalMonthOfQuarter,FiscalMonthOfYear,FiscalQuarterOfYear,FiscalYear,Holiday) ", _
 "VALUES (@ts,'{0}',{1},{2},{3},{4},{5},{6},{7},{8},{9},{10},{11},{12},{13},{14},{15},{16},{17},{18},'{19}');", _
 .CalendarDate, .CalendarDayOfWeek, .CalendarDayOfMonth, .CalendarDayOfYear, _
 .CalendarWeekOfMonth, .CalendarWeekOfYear, .CalendarMonthOfYear, .calendarYear, _
 .FiscalDayOfWeek, .FiscalDayOfMonth, .FiscalDayOfQuarter, .FiscalDayOfYear, _
 .FiscalWeekOfMonth, .FiscalWeekOfQuarter, .FiscalWeekOfYear, _
 .FiscalMonthOfQuarter, .FiscalMonthOfYear, .FiscalQuarterOfYear, .FiscalYear, .Holiday)

Too many things can go wrong:

• You might make a mistake in the {n} count
• You might make a mistake when aligning the quoted '{n}' items with the correct parameters

The problem is that you have too many things that have to be well aligned:

• the column list in the INSERT
• the column list in the format string
• the column list in the format params

Rewrite with good old-fashioned concatenation instead of Strings.Format. That way you eliminate one potential error vector. The result will also be more readable.

It might be a good idea to make the columns appear on their own lines, both when writing the list for INSERT and the values. That way you could easily copy-paste one of the lists in notepad and put side-by-side with the other to see where you missed something. (It can happen that you swear everything is well-aligned, but you just won't see the mistake until you actually break up the lines and put them side by side. Speaking from experience here, unfortunately.)

Answer 2

For fWeekOfMonth = 1 To 5
 GoSub ProcessWeekDays
Next

All I'm going to say is that you know better and this is neither cute nor clever. This is a slippery slope to Spaghetti Code.

GoSub is leftover from an ancient version of Visual Basic that didn't have subroutines. Use a proper subroutine or function.

For what it's worth, it appears to be a correct and clean use of GoSub. You just shouldn't be using it.