0

I get the following compiler warning when implementing the frameworks IComparer interface. I understand the reason why, but in my case I ensure that I initialize all values of the collection with an instance. How can I avoid the warning? I tried the bang operator but it doesn't seem to be allowed here. Maybe there is an alternative for collections which are known to have no null elements?

warning CS8767: Nullability of reference types in type of parameter 'x' of 'int FooComparer .Compare(Foo x, Foo y)' doesn't match implicitly implemented member 'int IComparer.Compare(Foo? x, Foo? y)' (possibly because of nullability attributes).

class FooComparer : IComparer<Foo>
{
    public int Compare(Foo x, Foo y)
    {
        return y.Bar() - x.Bar();
    }
}
codymanix
  • 28,510
  • 21
  • 92
  • 151
  • 2
    Does this answer your question? [IEquatable and \[AllowNull\]](https://stackoverflow.com/questions/60750386/iequatablet-and-allownull) –  Dec 18 '22 at 01:33

2 Answers2

0

Ok I got it solved. Need to declare the arguments as nullable as the contract needs, but in implementation have to use bang to tell what we expect:

class FooComparer : IComparer<Foo>
{
    public int Compare(Foo? x, Foo? y)
    {
        return y!.Bar() - x!.Bar();
    }
}

I wished that I could declare the non-nullability already in the API.

codymanix
  • 28,510
  • 21
  • 92
  • 151
  • 3
    Don't use `!` here. the caller could legally pass `null` for either parameter, or both, and you'd get a `NullReferenceException` for a completely valid call. Perform the appropriate `null` checks intead. `null` (no instance) is generally considered "less than" any instance, so return a negative value if `x` is `null`, unless `y` is also `null`, in which case they're equal so return `0`. – madreflection Dec 18 '22 at 02:49
  • @madreflection this is exactly my problem. there seems to be no way to get an implementation of that interface that says "I will not accept null". It would be great to use the bang operator like this: class FooComparer : IComparer – codymanix Dec 18 '22 at 17:16
  • You have to accept `null` because that's the contract the interface defines. A caller has no concept of the constraints that your implementation wants to place on them because it only sees the interface. – madreflection Dec 18 '22 at 17:18
  • `if (x is null) return y is null ? 0 : -1; if (y is null) return 1; return y.Bar() - x.Bar(); }` – madreflection Dec 18 '22 at 17:18
  • 1
    If you really, truly, absolutely know you'll never get `null`, `throw new ArgumentNullException("x or y", "I was so sure this would never happen.");` if either is `null`. The comparisons are cheap and you'll never hit the `throw` if you're right. The compiler is satisfied, and you'll get an exception if your expectations are somehow not met in the future. – madreflection Dec 18 '22 at 18:19
0

you can disable the warning if you are sure about nullability.

#pragma warning disable CS8767 // Nullability of reference types in type of parameter doesn't match implicitly implemented member (possibly because of nullability attributes).
    public int Compare(Foo x, Foo y)
#pragma warning restore CS8767 // Nullability of reference types in type of parameter doesn't match implicitly implemented member (possibly because of nullability attributes).

you can refer to this if you want to disable this on the project level.

Vivek Nuna
  • 25,472
  • 25
  • 109
  • 197
  • This is terrible advice. The caller could legally pass `null` and you'd get a `NullReferenceException`. Disabling warnings (and using `!`) means that you know better, and in this case, *you absolutely do not*. Anyone with a reference to the interface expects that `null` will be handled correctly because the parameters are declared nullable. OP is trying to apply restrictions that can only exist behind the veil; the interface's definition, which is all the caller sees, cannot surface these restrictions, so the interface's contract must be respected. – madreflection Dec 18 '22 at 17:50
  • @madreflection OP clearly mentioned that he/she knows that its never going to be null and just wants to get rid of the warning – Vivek Nuna Dec 18 '22 at 17:53
  • It's still a bad idea. I don't care how sure OP is. It's fragile, and posting it will contribute to more fragile code in someone else's project where they won't read the caveat or can't be as certain that they won't get null. As a community, we can write better code and we need to learn to do that instead. – madreflection Dec 18 '22 at 18:21
  • @madreflection agree with you – Vivek Nuna Dec 18 '22 at 18:23