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

UserForm to handle Date Inputs (Day,Month,Year)

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

Problem

Some fraction of a follow-up to The half-finished version.

What's changed: Added year as well as Day/Month. Added input Validation. Implemented a poor man's .EnableEvents = false for UserForms. Re-jigged the event heirarchy (Change year --> Repopulate Months or Days, Change Months --> Repopulate Days).

As always, all feedback welcomed.

In particular, if you were given this code to maintain, what would you be thinking as you read through it?

Initialisation and populating control values:

```
Option Explicit

Private Userform_EnableEvents As Boolean

Private Sub UserForm_Initialize()

Userform_EnableEvents = True

PopulateYearBox Me.UF_BankRec_cbx_Year

End Sub

Private Sub PopulateYearBox(ByRef yearBox As MSForms.ComboBox)
DisableFormEvents

Dim ixYear As Long

For ixYear = 2000 To Year(Now)
yearBox.AddItem ixYear
Next ixYear

EnableFormEvents
End Sub

Private Sub PopulateMonthBox(ByRef monthBox As MSForms.ComboBox, ByVal yearText As String)
DisableFormEvents

Dim ixYear As Long
ixYear = CLng(yearText)

Dim monthText As String
monthText = monthBox.Text

Dim ixMonth As Long, ixFinalMonth As Long
If ixYear = Year(Now) Then
ixFinalMonth = Month(Now)
Else
ixFinalMonth = 12
End If

monthBox.Clear
For ixMonth = 1 To ixFinalMonth
monthText = MonthName(ixMonth)
monthBox.AddItem monthText
Next ixMonth

EnableFormEvents
End Sub

Private Sub PopulateDayBox(ByRef dayBox As MSForms.ComboBox, ByVal monthText As String, ByVal yearText As String)
DisableFormEvents

Dim dateCounter As Date, startDate As Date
startDate = CDate("01/" & monthText & "/" & yearText)

dateCounter = startDate
dayBox.Clear
dayBox.AddItem Day(dateCounter)
dateCounter = dateCounter + 1
Do While Month(dateCounter) = Month(dateCounter - 1)
dayBo

Solution

This is going to make for either a poor UI or a change in behavior later. Better to just change its behavior now.

Dim ixYear As Long

      For ixYear = 2000 To Year(Now)
           yearBox.AddItem ixYear
      Next ixYear


The problem is that this is always and forever going to start on the year 2000. The list will just continue to grow over time. That's fine if you need the year 2000 to always be there, but you should ask your users if that's the case. I see two options here.

-
Calculate backwards 15 years.

Dim thisYear = Year(Now)
For ixYear = (thisYear - 15) To thisYear


-
Make this year the first option in the list and populate the list in reverse.

For ixYear = Year(Now) To 2000 Step -1


The approach you take will depend on your exact requirements. Of course, if you take the second route, you'll quickly find all the places where you hardcoded the number 2000.

The only other "issue" I see worth mentioning is this use of empty quotes.

If yearBox.Text <> "" Then


You should use vbNullString wherever possible. It's intent is more clear and it uses less memory than the empty string literal. (Okay, so it's a negligible amount of memory, but still...)

All in all its good code, very self documenting as far as what it does, but some comments explaining why couldn't hurt. For example, why does the selection start at the year 2000?

I've got to say, I'm impressed at how far you've come since your first post here.

Code Snippets

Dim ixYear As Long

      For ixYear = 2000 To Year(Now)
           yearBox.AddItem ixYear
      Next ixYear
Dim thisYear = Year(Now)
For ixYear = (thisYear - 15) To thisYear
For ixYear = Year(Now) To 2000 Step -1
If yearBox.Text <> "" Then

Context

StackExchange Code Review Q#114051, answer score: 3

Revisions (0)

No revisions yet.