23
\$\begingroup\$
public static DateTime ObjectToDateTime(object o, DateTime defaultValue)
{            
    if (o == null) return defaultValue;

    DateTime dt;
    if (DateTime.TryParse(o.ToString(), out dt))
        return dt;
    else
        return defaultValue;
}

The code feels too wordy and smells bad. Is there a better way?

\$\endgroup\$
  • 1
    \$\begingroup\$ You could get rid of the "else" (because in the true case it has already returned), but that's not much of an improvement. Other than that, this looks pretty optimal (clarity-wise) to me. \$\endgroup\$ – Carl Manaster Aug 4 '11 at 17:55
  • \$\begingroup\$ return o as DateTime ?? defaultValue; \$\endgroup\$ – user6339 Aug 15 '11 at 4:36
  • \$\begingroup\$ DateTime is a value type and thus could not be used with the as operator nor set to null. That would be invalid code. \$\endgroup\$ – Jeff Mercado Aug 15 '11 at 4:42
  • \$\begingroup\$ As @Jeff Mercado says, you cannot use a DateTime object with the as keyword. A failed conversion cannot be set to null. However, if you use a DateTime? (nullable DateTime) instead, your code would work. \$\endgroup\$ – Jason Down Aug 16 '11 at 1:54
30
\$\begingroup\$

On the offhand chance that your object is already a DateTime, you're performing unnecessary conversions to and from strings.

 if (o is DateTime)
     return (DateTime)o;

This also strikes me as something you might be doing for a database item, for example. In which case, I'd encourage you to know and trust your data types and then use existing methods of retrieval.

For example, if you have a DataTable with a column CreatedDate, you should know it's a date, what you might not know is if it has a value if the column is nullable at the database. That's fine, you can handle that in code.

 var createdDate = row.Field<DateTime?>("CreatedDate"); 

There we go, a DateTime?, no coding of a conversion necessary. You can even specify a default and type if to DateTime

 var createdDate = row.Field<DateTime?>("CreatedDate").GetValueOrDefault(DateTime.Now);
 var createdDate = row.Field<DateTime?>("CreatedDate") ?? DateTime.Now;
|improve this answer|||||
\$\endgroup\$
3
\$\begingroup\$

I like @ChaosPandion's solution however I find the following a bit quicker to read.

public static DateTime ObjectToDateTime(object o, DateTime defaultValue)
{    
    DateTime result;
    if (DateTime.TryParse((o ?? "").ToString(), out result)) {
        return result;
    } else {
        return defaultValue;
    }
}

If you prefer the explicit null check, modify the if statement a bit...

    if (o != null && DateTime.TryParse(o.ToString(), out result)) {

Coming from a language without the ?? operator, I find this even easier to quickly read and understand the intent of however I recognize that is probably just because I'm not used to reading ??.

|improve this answer|||||
\$\endgroup\$
3
\$\begingroup\$

Side note

Since C# 7 it is now possible to declare the variables within the is clause which can come handy. For example

        if (o1 is DateTime startDate &&
            o2 is DateTime endDate &&
            startDate > endDate)
            return true;
|improve this answer|||||
\$\endgroup\$
  • 1
    \$\begingroup\$ And to match OP code you may also show it with out var dt in TryParse() \$\endgroup\$ – Adriano Repetti Jul 24 '17 at 16:45
1
\$\begingroup\$

The big problem I see here is your method name. I'd expect ObjectToDateTime to parse my object to a DateTime but that's not exactly what happens, right? Your method tries to parse the input and returns a default value if this doesn't work. Your method name should reflect that sentence. I'd call it ParseDateTimeOrDefault or something like that. It kind of follows the IEnumerable.SingleOrDefault trend.

Now, there's another problem. Your intention seems to be to cast an object to a DateTime, but you actually parse the ToString() of the object.

The point is, as a user of your method, I'd expect this to return the default value :

DateTime someDateTime; /*= Unimportant*/
dt = DateTime.Now;
ObjectToDateTime(dt.ToString(), someDateTime);

Why? Because the object I pass as a parameter isn't a DateTime, it's a string. You parse a string, if it doesn't work, you return a value. That's your method.

The question is : In your application, is there a scenario where you don't know if you'll either receive a string or a DateTime? Because that seems like a design flaw in my opinion.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.