Help with a function I found in a VB.NET 2.0 app I am maintaining

Question

I was going through some reports that were supposed to be displaying 2/1/2009 - 2/28/2009, but instead was displaying 2/1/2009 - 3/1/2009. I have narrowed the bug down to this code, any suggestions?

Function GetMonthLastDate(ByVal sDateTime As DateTime)
    Try
        Dim strArrMonth() As String = {"", "31", "29", "31", "30", "31", "30", "31", "31", "30", "31", "30", "31"}
        Dim days As Integer
        If sDateTime.Month = 2 Then
            If sDateTime.Year Mod 400 = 0 Then Return sDateTime.AddDays(28)
            If sDateTime.Year Mod 100 = 0 Then Return sDateTime.AddDays(27)
            If sDateTime.Year Mod 4 = 0 Then Return sDateTime.AddDays(28)
        End If
        days = strArrMonth(sDateTime.Month)
        Return Format(sDateTime.AddDays(days - 1), "MM/dd/yyyy")
    Catch ex As Exception
        Response.Write("<script>alert('" & ex.Message & "');</script>")
    End Try
End Function

Answer 1

This is some seriously convoluted code for what it's doing. You can accomplish the same thing like this:

Function GetMonthLastDate(ByVal sDateTime As DateTime)
  Dim nextMonth As DateTime = sDateTime.AddMonths(1)
  Return New DateTime(nextMonth.YearPart, nextMonth.MonthPart, 1).AddDays(-1)
End Function

That might be a little cryptic, but it should work and can be expanded for clarity.

Answer 2

Why not use DateTime.DaysInMonth? It makes the code much more clear.

Function GetMonthLastDate(ByVal srcDate As DateTime) As String
    return _
       new DateTime(srcDate.Year, srcDate.Month, _
                    DateTime.DaysInMonth(srcDate.Year, srcDate.Month)) _
           .ToString("MM/dd/yyyy")
End Function

You could just as easily (and probably ought to) return the DateTime itself (by changing the return type and nixing the .ToString(...)), but I see your original function returned it as a formatted string, so for compatibility purposes I've kept it the way you had it.

Answer 3

In the lookup array, index 2 (valued at 29) is only right for leap years.

That whole function is just bad: it's way over-complicated, returns an incorrect result, doesn't specify the return type, and not only has a type prefix on the parameter name but it's the wrong prefix.

Do it like this instead:

Function GetLastMonthDate(ByVal dtDateTime As DateTime) As DateTime
    Return dtDateTime.AddDays(DateTime.DaysInMonth(dtDateTime.Year, dtDateTime.Month)-dtDateTime.Day)
End Function

Beyond this, it appears the original returns a string, which is really not appropriate. This function should return a DateTime and let the function that calls it handle the appropriate string conversion.