Dictionary lookup of non-nullable struct into nullable destination

General

I think I would question the value of this extension method. You've already got the same information from the existing TryGetValue method:

• Could the key be found?
• What is the value?

If at all possible, I'd recommend trying to modify the calling code not to need this extension:

DateTime triedNearestDateUtc; var nearestDateUtc = nearestDatesUtc.TryGetValue(company, out triedNearestDateUtc) ? (DateTime?)triedNearestDateUtc : null; 

or even do away with the nullable entirely:

DateTime triedNearestDateUtc; if (nearestDatesUtc.TryGetValue(company, out triedNearestDateUtc)) { // Do something with value } else { // Do something without value } 

If there are multiple possible sources for your value (which your if (!nearestDatesUtc.TryGetValue(...)) suggests), try:

DateTime nearestDateUtc; var foundNearestDateUtc = nearestDatesUtc.TryGetValue(company, out nearestDateUtc); if (!foundNearestDateUtc) { foundNearestDateUtc = otherDatesUtc.TryGetValue(company, out nearestDateUtc); } if (!foundNearestDateUtc) { nearestDateUtc = DateTime.Now; } 

Or break that part into its own method:

private DateTime FindNearestDateUtc(string company) { DateTime nearestDateUtc; if (nearestDatesUtc.TryGetValue(company, out nearestDateUtc)) { return nearestDateUtc; } if (otherDatesUtc.TryGetValue(company, out nearestDateUtc)) { return nearestDateUtc; } return DateTime.Now; } 

But this is all a bit speculative on what you're using it for. Onto the bits you've actually put in your question:

DateTime

The DateTime type is a tricky customer - it tries to be all things to all people. I understand if it'd be a lot of effort and upheaval to move away from this type now, but I'd strongly recommend NodaTime as an alternative - it forces you to think clearly about exactly what kind of date and time structures you mean, and helps to prevent subtle bugs down the line.

Some interesting reading (if you find this sort of thing interesting) on the subject: Jon Skeet on DateTime

Signature

The main reason for having the bool return value as well as the out parameter is that default(DateTime) could be a valid return value. With a signature like DateTime TryGetValue(string key), you have no way of knowing whether 0001-01-01T00:00:00 was the value or the default "I don't have a value" value.

In your case, you seem to be using a null DateTime? to indicate "I don't have a value" and a "real" DateTime? to indicate the result. There's no ambiguity here, so there's no reason you can't just return the value on its own.

This also has a nice side effect of making the body of the method a bit shorter and simpler:

 TValue result; if (that.TryGetValue(key, out result)) { return result; } else { return null; } 

or

 TValue result; return that.TryGetValue(key, out result) ? (TValue?)result : null; 

Naming

• I'd probably give the method a rename because it behaves differently from the normal TryGetValue: Try... methods conventionally have a bool return and an out result parameter.

• that isn't a good name for a variable - it doesn't tell you anything about what it is or does.

public static TValue? GetValueOrNull<TKey, TValue>( this IDictionary<TKey, TValue> dictionary, TKey key) where TValue : struct { ... } 

Result

I'd go with the ternary operator over the if ... else version and end up with:

public static TValue? GetValueOrNull<TKey, TValue>( this IDictionary<TKey, TValue> dictionary, TKey key) where TValue : struct { TValue result; return dictionary.TryGetValue(key, out result) ? (TValue?)result : null; }