9

I'm writing up a generic method that needs to internally use a Dictionary keyed by the generic parameter.

I don't want to impose any constraint on the generic parameter itself, in particular no notnull constraint. However, the method would be capable of handling nulls explicitly, and of course not adding any null keys to the dictionary it's using.

Is there any best practice for doing so?

By default, the compiler warns that the generic key type doesn't match the notnull constraint of Dictionary<,>.

See an example below. Unless I'm missing something, this should be perfectly safe. But how can I best convey this to the compiler? Is there any way other than suppressing the compiler warning here?

public static int CountMostFrequentItemOrItems<T>(params T[] values)
{
    var counts = new Dictionary<T, int>();  // The type 'T' cannot be used as type parameter 'TKey' in the generic type or method 'Dictionary<TKey, TValue>'
    var nullCount = 0;
    
    foreach(var value in values)
        if (value is null)
            ++nullCount;
        else if (counts.TryGetValue(value, out var count))
            counts[value] = count+1;
        else
            counts[value] = 1;

    // ... combine results etc ...
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Bogey
  • 4,926
  • 4
  • 32
  • 57
  • 1
    see https://stackoverflow.com/questions/4632945/why-doesnt-dictionarytkey-tvalue-support-null-key – CthenB Nov 23 '22 at 10:08
  • Does this answer your question? [Why doesn't Dictionary support null key?](https://stackoverflow.com/questions/4632945/why-doesnt-dictionarytkey-tvalue-support-null-key) – CthenB Nov 23 '22 at 10:09
  • 6
    @CthenB Neither of those answer the question. OP knows that dictionaries don't support null keys -- the question is very clear on this. The question is about now to structure NRT annotations to tell the compiler that even though `T` is nullable, the dictionary will never be given a null key – canton7 Nov 23 '22 at 10:10
  • I'm not even sure whether it's possible to suppress this... – canton7 Nov 23 '22 at 10:13
  • 3
    @canton7 I think suppressing by "#pragma warning disable/restore CS8714" works .. but is a little ugly! – Bogey Nov 23 '22 at 10:16
  • @canton7 the linked post has a most-upvoted answer showing how to bypass the issue by creating a custom implementation which handles the null separately. – CthenB Nov 23 '22 at 10:17
  • 2
    As a side note, the `CountMostFrequentItemOrItems` method could be optimized with the [`CollectionsMarshal.GetValueRefOrAddDefault`](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.collectionsmarshal.getvaluereforadddefault) API ([usage example](https://stackoverflow.com/questions/74083010/value-tuples-exist-so-why-use-the-out-parameter-modifier/74083392#74083392)). – Theodor Zoulias Nov 23 '22 at 10:18
  • 1
    @CthenB That describes how do create a new type to use as the key, which effectively hacks in support for null keys. That is one potential workaround, but it doesn't directly answer the question – canton7 Nov 23 '22 at 10:18
  • 1
    Maybe not topic of that question, but I wouldn't solve this problem with a `Dictionary<>` at all. I would use `GroupBy` to group, then `Where` to remove any null, and then a `Max` in combination with `Count` to get the most frequent item. Should be a oneliner. – Matthias Auswöger Nov 23 '22 at 10:18
  • 2
    @MatthiasAuswöger the `GroupBy` is an inefficient solution for this problem. What you need is the [`CountBy`](https://github.com/dotnet/runtime/issues/77716), which is currently just a proposal. – Theodor Zoulias Nov 23 '22 at 10:21
  • 1
    It's worth noting that MoreLinq's implementation of CountBy doesn't support null keys – canton7 Nov 23 '22 at 10:24
  • Dictionary has a constraint `where TKey : notnull`, so if you can bypass it, you also break the c# constraints. – shingo Nov 23 '22 at 10:41
  • I only get the warning when project has `enable` – Magnus Nov 23 '22 at 10:42
  • @Magnus Yeah, that's the default now for .Net 7/C#11 (for new projects) – Matthew Watson Nov 23 '22 at 10:48
  • How does the full function look? – Magnus Nov 23 '22 at 10:50
  • 2
    The "proper" solution would be to support some variation of `MaybeNull`/`NotNull` on the type parameter itself (so *not* as a constraint), as that's the usual way to work around situations where you know better than the compiler, but .NET doesn't currently offer this (support for attributes on type parameters was added at some point, though, so this *could* be done). Suppressing the warning is the correct approach here -- that's why nullability analysis is warnings in the first place, since we *know* the compiler can't do this perfectly with the approach C# has chosen. – Jeroen Mostert Nov 23 '22 at 11:16
  • 1
    Yep totally agree with Jeroen - just suppress the warning. – Matthew Watson Nov 23 '22 at 11:20
  • 2
    Thanks @JeroenMostert . I wasn't sure if I missed anything, but looks like that's unfortunately not the case. Feel free to post this as the answer to this question then; an ugly solution is still better than no solution! – Bogey Nov 23 '22 at 11:52
  • The ability to write `new Dictionary()` would be yet another "proper" solution. – Olivier Jacot-Descombes Nov 29 '22 at 11:32

0 Answers0