Remove last ! from Caching.Abstractions
#85918
Conversation
msftbot
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-extensions-caching Issue DetailsClose #64147
|
| @@ -174,7 +174,8 @@ public static ICacheEntry SetOptions(this ICacheEntry entry, MemoryCacheEntryOpt | |||
|
|
|||
| foreach (PostEvictionCallbackRegistration postEvictionCallback in options.PostEvictionCallbacks) | |||
| { | |||
| entry.RegisterPostEvictionCallback(postEvictionCallback.EvictionCallback!, postEvictionCallback.State); | |||
| ArgumentNullException.ThrowIfNull(postEvictionCallback.EvictionCallback); | |||
There was a problem hiding this comment.
Why is this a beneficial change? It seems like it's adding another check-and-throw that's just duplicating what RegisterPostEvictionCallback is about to do, anyway.
There was a problem hiding this comment.
I'm not opposite to simply remove !
There was a problem hiding this comment.
Here is a small discussion #64147 (comment)
There was a problem hiding this comment.
I'm not opposite to simply remove !
EvictionCallback is typed as PostEvictionDelegate?. RegisterPostEvictionCallback is typed to accept PostEvictionDelegate. So we can't just remove the !. I'm just not clear on why we need to change anything here, if all we're going to do is throw the same exception.
There was a problem hiding this comment.
If it's fine to just pass null to the next check in RegisterPostEvictionCallback, I'll update it
There was a problem hiding this comment.
I'll update it
Update it to what? The ! is necessary to suppress the warning about passing in something that's possibly null to something that requires not-null, right?
From the discussion you linked to, it sounds like the goal of the issue was to review !s in the library and this was the last one. This one is actually necessary, so I'd suggest we just close that issue and this PR.
But I'll leave it for @eerhardt to decide, since he was part of that linked conversation.
Close #64147