dropdownlist .net

Question

this is my code in vb.net -

Public Sub Load_dates(ByRef DDL As System.Web.UI.WebControls.DropDownList)
    Try
        Dim i As Integer
        Dim j As Integer
        Dim time As DateTime = DateTime.Now
        For i = Now.Year To Now.Year
            For j = 0 To 11
                DDL.Items.Add((time.AddMonths(j)).ToString("MMMM") + " " + (i.ToString))
            Next
        Next
    Catch ex As Exception
        ReportError(ex)
    End Try
End Sub

After I get all the months, 0 to 11, i want to pick up the 11th record and add that as the last record with a "+" in the end. e.g - Last record = December 2011, then the record after that should be December 2011 +

Answer 1

here is an ugly solution for you...

Public Sub Load_dates(ByRef DDL As System.Web.UI.WebControls.DropDownList)
   dim last as string
   last = ""
    Try
        Dim i As Integer
        Dim j As Integer
        Dim time As DateTime = DateTime.Now
        For i = Now.Year To Now.Year
            For j = 0 To 11
                last = (time.AddMonths(j)).ToString("MMMM")
+ " " + (i.ToString)
                DDL.Items.Add(last)
            Next
        Next
    Catch ex As Exception
        ReportError(ex)
    End Try
    if not last = "" then DDL.Items.Add(last+"+") 
 End Sub

note I did not test there might be typos.

Answer 2

This code hurts on many levels. I will answer your question, but first some tips:

• Parameters should have nice names. "DDL" is meaningless to me. Furthermore, they should not be passed by reference unless absolutely necessary. You aren't modifying DLL (only what it points to), so pass it by value.
• Methods should, when possible, not have side-effects. By passing in the drop down list, instead of just returning entries for it, you are introducing side-effects. This also couples the method to the particular drop down control type.
• The declarations of i and j are superfluous (assuming Option Infer On), and push their scope outside of the loops.
• Why is i looping over the current year? You've introduced some time-sensitive bugs by doing this (loop could run twice if executed new year's eve at midnight).
• You're adding months to 'time', but the printed year is constant. You're going to see "January 2009" following "December 2009"!
• Why are you catching such a general exception within such a simple method? I don't see any calls which could conceivably throw an exception you'd want to report at this level.
• What if you want to change how many months to project forwards? The '12' is implicitely hidden deep within the For-Loop!

All in all I have to say this is very poor code. But it is not hard to improve it step by step. After applying my own advice to it, I ended up with this:

Public Function GenerateMonthEntries(monthCount as integer) as IList(of String)
    dim result = new List(Of String)()
    dim time = DateTime.Now
    for i = 0 to monthCount - 1
        result.add(time.AddMonths(i).ToString("MMMM yyyy"))
    next i
    result.add(time.AddMonths(monthCount).ToString("MMMM yyyy") + "+")
    return result
End Function

... where you're using the function ...

for each entry in GenerateMonthEntries(12) 'maybe the 12 should be a constant somewhere
    whateverDropDownList.Items.add(entry)
next

Don't be fooled though, this code is not perfect. Maybe the output type should be the more general IEnumerable(Of String). Maybe the 'time' variable should be a parameter instead, so you can easily test the method and make its true dependencies obvious.