Dictionary lookup of non-nullable struct into nullable destination

Question

I've got an IDictionary<SomeT, DateTime> that does not always contain the desired key-value pair. For that reason, a target method expects a DateTime? that the default TryGetValue method cannot provide a value for directly, because of the added ?.

I've written this generic extension with main emphasis on the parameter out TValue? value.

public static bool TryGetValue<TKey, TValue>(
 this IDictionary<TKey, TValue> that,
 TKey key,
 out TValue? value) where TValue : struct
{
 TValue output;
 if (that.TryGetValue(key, out output))
 {
 value = output;
 return true;
 }
 else
 {
 value = null;
 return false;
 }
}

Example use:

DateTime? nearestDateUtc; // The standard TryGetValue would not allow '?' here.
if (!nearestDatesUtc.TryGetValue(company, out nearestDateUtc)) { ... }

Based on this, my question is then; is there a more idiomatic way to extract (non-null) structs into nullable destinations than the solution I implemented? And if not, is the name TryGetValue confusing? C# developers might expect something very specific about exactly this name. In that case, what would a better name be?

An error could easily occur as the overload comes down to the presence of a single question-mark in a variable's type declaration and would mean the difference between null and default(DateTime).

• Jeroen Vannevel: I don't see anything that points to stub code. This just how generics look
• Comintern: I'm not sure I understand the purpose of the extension. Is it so you can treat a DateTime and DateTime? as if they are the same type? If that's the case, I'd question the calling code more than the implementation that allows it.
• sshine: @Malachi: This is real code. @Comintern: I've demonstrated the purpose in the added example. Basically, I want a null value when the dictionary does not contain this, rather than the value default(DateTime), which is something like 01-01-0001 00:00:00. But I lack the foresight of whether this approach is good or if it would introduce subtle bugs.

Answer 1

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月01日T00: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;
}

Answer 2

Naming

TryGetValue is not the best name, because like you already mentioned any developer which is familiar with the Dictionary<TKey, TValue> would get confused. How about TryGetValueOrNull?

General

I don't like the idea to get a nullable if the key isn't found. This is clearly signaled by the returned value of the default TryGetValue() method of the Dictionary.

Basically it doesn't buy you very much.

Style

• The else in the method is redundant and can be removed.

• You should always check the passed this parameter of an extension method wether it is null because such a method can be called like any other static method directly without a reference of the this type.

• that is not a good parameter name.

Summing up, this will lead to

public static bool TryGetValueOrNull<TKey, TValue>(
 this IDictionary<TKey, TValue> dictionary,
 TKey key,
 out TValue? value) where TValue : struct
{
 if (dictionary == null) { throw new ArgumentNullException(nameof(dictionary)); }
 TValue output;
 if (dictionary.TryGetValue(key, out output))
 {
 value = output;
 return true;
 }
 value = null;
 return false;
}