Sometimes when maintaining old code I stumble into situations where people are really going to too much effort to get something done. For whatever reason, this is particularly common with date math. Consider this VB.NET code, which not only is doing things the hard way, but has a bug that will occur once every four years. Can you spot it?
dtpStart.Value = (Now.Month).ToString & "/1/" & Now.Year.ToString
Select Case Now.Month Case 1, 3, 5, 7, 8, 10, 12 : tDay = "31"
Case 2
If Now.Year / 4 = 0 Then
tDay = "29"
Else
tDay = "28"
End If
Case 4, 6, 9, 11 : tDay = "30"
End Select
dtpEnd.Value = (Now.Month).ToString & "/" & tDay & "/" & Now.Year.ToString
Here we’re trying to come up with the first and last days of the current month. The original code thought of leap year, at least, but did a division instead of a modulus, so “Now.Year / 4” should be “Now.Year Mod 4”. As far as that goes, the 400-year exception was overlooked, too. But why clutter your brain with all that stuff? The same thing can be accomplished in two lines of code, and without all the string concatenations:
dtpStart.Value = Today.AddDays(1 - Today.Day) dtpEnd.Value = dtpStart.Value.AddMonths(1).AddDays(-1)
Remember, a lazy programmer is a good programmer!
Update: For those of you who never read comments, here is the above code with suggested improvements in clarity made by several posters. I’ve sacrificed an extra line to declare the variable d, in order to make the other two lines more readable. Plus, avoiding VB.NET’s built-in Today variable makes the code easier to convert to C#, as well as increasing our knowledge of the CLR rather than burying it under syntax sugar that makes us dependent on a particular language.
Dim d As DateTime = DateTime.Today dtpStart.Value = New DateTime(d.Year,d.Month,1) dtpEnd.Value = New DateTime(d.Year,d.Month,DateTime.DaysInMonth(d.Year,d.Month))
{ 5 comments… read them below or add one }
While I agree with dtpEnd, dtpStart is a little obfuscated. Wouldn’t you see the same benefit with added clarity with something like…
dtpStart.Value = new DateTime(DateTime.Now.Year, DateTime.Now.Month, 1)
Bob responds: Yep, your way is more self-evident. Self-evident is good!
bah, knee-jerk post, didn’t see the assumed “Today” variable, but adds even shorter goodness:
dtpStart.Value = new DateTime(Today.Year, Today.Month, 1)
Bob responds: You sound like a fellow C# guy. Today is baked into the Microsoft.VisualBasic namespace. When I first saw “Now” in the original code, I automatically used “Today” assuming that it was VB-speak for “DateTime.Today”. Actually as I’ve mentioned elsewhere, I’d personally stick with DateTime.Today even in VB.NET, though.
How about this:
I like the Days in month feature as well.
Where Year = some integer and
Month is another integer
DateTime.DaysInMonth(Year, Month)
Bob responds: Thanks, Matt! All these great suggestions underscore that you should not ignore the wealth of resources in the CLR by focusing only on legacy language features! Seriously, I don’t think the author of the original code I showed was stupid or careless; but they were stuck in a rut (in this case, a legacy VB6 / VBScript rut). Paradoxically, it may have seemed easier to the coder to do it the hard way, because it would have cost as much or more to stop and look up what was available. But look at the benefits that would have flowed from that little interlude of discovery.
I agree. In fact someday our code will probably look rather outdated to some other more modern language. Who knows maybe in C# 5.0 you will just type DateTime.February or something similar :)
Gives u the last day of the month
DateTime.DaysInMonth(DateTime.Today.Year, DateTime.Today.Month)