Bad code bites in an unexpected way

I've always known bad code bites you back in a number of ways, but this one was new. See the following code snippet:

string lastHeading = DateTime.MinValue.ToShortDateString();
foreach (Item i in GetItems()) {
if (i.Date.ToShortDateString() != lastHeading) {
lastHeading = i.Date.ToShortDateString();
Console.WriteLine("  " + i.Text);

So what does it do? Quite nicely, it takes care of printing date headers for printed items in date order. That is, under the heading "2005-09-14" are printed the entries related to that particular date.

Now, you could ask, "What's wrong with that code then?" Yes, that's not entirely obvious. The issue here is that comparisons related to the "lastHeading" are string comparisons when in fact it's DateTimes being compared. Thus, this example violates the rule of "Always use the most correct and exact data type". However, the current approach could also be argued for by saying it's actually the "heading semantics" that rule here, thus making strings the correct data type – but I won't be going there. Not now.

Well, why did this sort of structure from somebody's code fail on me today? Believe it or not, it was because of the initialization to DateTime.MinValue.ToShortDateString(). Now, there's nothing fundamentally wrong with that – it's a surefire way to force the header to be printed (although setting the string to empty or null would be more logical here if we consider this to be purely string data).

Well, we got an ArgumentNullException with an interesting error message. Why's that? Because somebody ran the software using Arabic culture (ar-SA), which uses Hijri calendar system. Unfortunately Hijri calendar doesn't have a way to format anything as low as DateTime.MinValue. Uh… What a really awkward reason to fail.

The moral of the story: Date operations (DateTime, TimeSpan et al.) are by nature culture independent. Display formatting for them rarely is, so never assume too much. And use the correct data types.

September 14, 2005 В· Jouni Heikniemi В· 2 Comments
Posted in: .NET

2 Responses

  1. Juha-Mikko - September 15, 2005

    If the DateTime class supports localization, why doesn't MinValue respect the locale? I see nothing wrong in assuming a supposedly locale-aware class to return meaningful values for the selected locale.

  2. Jouni - September 16, 2005

    It is somewhat debatable whether or not the DateTime class should be locale-aware by its value conventions. Probably it shouldn't – in essence, it _is_ a "universal" data type similar to int etc., which are useful even in a culture whose numeric system couldn't represent negative values (just as an example).
    Also, the concept of "DateTime.MinValue" isn't particularly useful in a calendar-specific concept – its most reasonable application is indicating some sort of a "null" or "empty" value and thus, any formatting should be special-cased anyway.
    If DateTime value range was limited to the calendar representation range of the current thread culture, it would pose some interesting questions. For example, what would happen to a DateTime that's valid in culture A when switching the current thread culture to B, under which the given date wouldn't validate?