Dictionary lookup of non-nullable struct into nullable destination

I've got an IDictionary 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(
    this IDictionary 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).

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(
    this IDictionary 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(
    this IDictionary dictionary,
    TKey key) where TValue : struct
{
    TValue result;
    return dictionary.TryGetValue(key, out result) ? (TValue?)result : null;
}

StackExchange Code Review Q#117102, answer score: 4