6
\$\begingroup\$

So I have a simple userform where I require the User to input the target month, and the day to analyse data up to.

This is just a small section of code to govern populating the month selection and then determining days based on the month.

Particular questions:

Is this a smart way to handle dates generally?
Is this a good way to ensure that the User cannot enter a non-existent date?
Any edge cases I haven't considered?

And, as always, if you were handed this code to maintain, what would you be thinking as you read through it?

Private Sub UserForm_Initialize()
 PopulateMonthBox Me.UF_BankRec_cbx_Month
End Sub
Private Sub PopulateMonthBox(ByRef monthBox As MSForms.ComboBox)
 Dim ix As Long
 Dim monthText As String
 For ix = 1 To 12
 monthText = MonthName(ix)
 monthBox.AddItem monthText
 Next ix
End Sub
Private Sub UF_BankRec_cbx_Month_Change()
 Dim dayBox As MSForms.ComboBox
 Set dayBox = Me.UF_BankRec_cbx_EndDay
 Dim monthBox As MSForms.ComboBox
 Set monthBox = Me.UF_BankRec_cbx_Month
 Dim monthText As String
 monthText = monthBox.Text
 dayBox.Clear
 PopulateDayBox dayBox, monthText
End Sub
Private Sub PopulateDayBox(ByRef dayBox As MSForms.ComboBox, ByVal monthText As String)
 Dim ix As Long, dayLimit As Long
 Dim ixMonth As Long, ixNextMonth As Long
 Dim ixYear As Long
 Dim firstDayNextMonth As Date, lastDayThisMonth As Date
 ixMonth = Month("01/" & monthText & "/2000")
 ixNextMonth = ixMonth + 1
 ixYear = Year(Now)
 firstDayNextMonth = DateSerial(ixYear, ixNextMonth, 1)
 lastDayThisMonth = DateAdd("d", -1, firstDayNextMonth)
 dayLimit = Day(lastDayThisMonth)
 For ix = 1 To dayLimit
 dayBox.AddItem ix
 Next ix
End Sub
asked Dec 14, 2015 at 11:34
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Why not doing it more directly like this:

Private Sub UserForm_Initialize()
 Dim i As Byte
 For i = 1 To 12
 UF_BankRec_cbx_Month.AddItem MonthName(i)
 Next
End Sub
Private Sub UF_BankRec_cbx_Month_Change()
 Dim i As Byte, startDate As Long
 UF_BankRec_cbx_EndDay.Clear
 startDate = DateValue("01/" & UF_BankRec_cbx_Month.Text & "/" & Year(Now()))
 While Month(startDate) = Month(startDate + i)
 i = i + 1
 UF_BankRec_cbx_EndDay.AddItem i
 Wend
End Sub

I'd say it is a bit better to read while i normally just speed codes up (so they end up in a way you can't read them anymore)... I tried my best to make it readable even without comments. However, there may be a reason for you to run extra macros and using objects which i skipped out here... (to insert arrays to CBoxes is also possible solution to avoid objects)

If there any questions or something like that, feel free to ask Or better, tell me what you don't like looking at my code (this way i get the best idea what you desire) :)

At least your lastDayThisMonth = DateAdd("d", -1, firstDayNextMonth) is the same like lastDayThisMonth = firstDayNextMonth - 1 (but i am sure there is also a reason for that)

answered Dec 14, 2015 at 14:27
\$\endgroup\$
6
  • \$\begingroup\$ I like the more direct day-looping. That's pretty neat. But, I would hate to have a whole project written (more specifically, named) like that. dim i as (counter), dim j as (dateValue). Yeah, that's going to get very difficult to follow. \$\endgroup\$ Commented Dec 14, 2015 at 14:56
  • \$\begingroup\$ Also, I had no idea VBA even had a byte DataType :p \$\endgroup\$ Commented Dec 14, 2015 at 15:01
  • 1
    \$\begingroup\$ you are completely right regarding j so i renamed it to startDate to make it more obvious... but looking at i it is like ab big red arrow telling me it counts (also most other ppl use it that way)... but at the end you are the one who counts for real... if you feel better using different variable names, just do it :D (but if you ever need to read code from other ppl, remember the big red arrow for i, j, k....) \$\endgroup\$ Commented Dec 14, 2015 at 15:25
  • \$\begingroup\$ Oh, single-letter-variable == counter I'm very familiar with. It's using it for anything else I object to :) \$\endgroup\$ Commented Dec 14, 2015 at 15:35
  • \$\begingroup\$ If you have another VBA code to be checked, just show it :D \$\endgroup\$ Commented Dec 14, 2015 at 15:59

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.