views:

3484

answers:

6

This may seem rudimentary to some, but this question has been nagging at me and as I write some code, I figured I would ask.

Which of the following is better code in c# and why?

((DateTime)g[0]["MyUntypedDateField"]).ToShortDateString()

or

DateTime.Parse(g[0]["MyUntypedDateField"].ToString()).ToShortDateString()

Ultimately, is it better to cast or to parse? Thanks all!

+9  A: 

If g[0]["MyUntypedDateField"] is really a DateTime object, then the cast is the better choice. If it's not really a DateTime, then you have no choice but to use the Parse (you would get an InvalidCastException if you tried to use the cast)

Wilka
When the object is not a DateTime, don't use Parse, rather use ParseExact and specify the expected date and time format, to avoid being dependent on current user culture settings. See my answer on this question.
qbeuek
A: 

As @Brian R. Bondy pointed it depends on implementation of g[0]["MyUntypedDateField"]. Safe practice is to use DateTime.TryParse and as operator.

aku
Let's avoid writing defensive code. The developer should know what type / format to expect and **fail** (eg. throw an exception) when the value supplied is with a different format or type.TryParse should be used **only** on user supplied input.
qbeuek
"Let's avoid writing defensive code" heard it million times. Usually it ends up as "Damn! they changed internal implementation!"
aku
A: 

Parse requires a string for input, casting requires an object, so in the second example you provide above, then you are required to perform two casts: one from an object to a string, then from a string to a DateTime. The first does not.

However, if there is a risk of an exception when you perform the cast, then you might want to go the second route so you can TryParse and avoid an expensive exception to be thrown. Otherwise, go the most efficient route and just cast once (from object to DateTime) rather than twice (from object to string to DateTime).

Ryan Farley
A: 

There's comparison of the different techniques at http://blogs.msdn.com/bclteam/archive/2005/02/11/371436.aspx.

sgwill
+2  A: 

Casting is the only good answer.

You have to remember, that ToString and Parse results are not always exact - there are cases, when you cannot safely roundtrip between those two functions.

The documentation of ToString says, it uses current thread culture settings. The documentation of Parse says, it also uses current thread culture settings (so far so good - they are using the same culture), but there is an explicit remark, that:

Formatting is influenced by properties of the current DateTimeFormatInfo object, which by default are derived from the Regional and Language Options item in Control Panel. One reason the Parse method can unexpectedly throw FormatException is if the current DateTimeFormatInfo.DateSeparator and DateTimeFormatInfo.TimeSeparator properties are set to the same value.

So depending on the users settings, the ToString/Parse code can and will unexpectedly fail...

qbeuek
+1  A: 

Your code suggests that the variable may be either a date or a string that looks like a date. Dates you can simply return wit a cast, but strings must be parsed. Parsing comes with two caveats;

  1. if you aren't certain this string can be parsed, then use DateTime.TryParse().

  2. Always include a reference to the culture you want to parse as. ToShortDateString() returns different outputs in different places. You will almost certainly want to parse using the same culture. I suggest this function dealing with both situations;

      private DateTime ParseDateTime(object data)
      {
       if (data is DateTime)
       {
        // already a date-time.
        return (DateTime)data;
       }
       else if (data is string)
       {
        // it's a local-format string.
        string dateString = (string)data;
        DateTime parseResult;
        if (DateTime.TryParse(dateString, CultureInfo.CurrentCulture, DateTimeStyles.AssumeLocal, out parseResult))
        {
         return parseResult;
        }
        else
        {
         throw new ArgumentOutOfRangeException("data", "could not parse this datetime:" + data);
        }
       }
       else
       {
        // it's neither a DateTime or a string; that's a problem.
        throw new ArgumentOutOfRangeException("data", "could not understand data of this type");
       }
      }
    

Then call like this;

ParseDateTime(g[0]["MyUntypedDateField").ToShortDateString();

Note that bad data throws an exception, so you'll want to catch that.

Also; the 'as' operator does not work with the DateTime data type, as this only works with reference types, and DateTime is a value type.

Steve Cooper