Use TryGetValue() for dictionary lookup #44791
Conversation
Use `TryGetValue()` instead of `ContainsKey()` and the indexer.
martincostello
requested review from
Tratcher,
BrennanConroy and
halter73
as code owners
ghost
added
the
community-contribution
|
Thanks for your PR, @martincostello. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Oh nice! Want to try enabling it (different PR is fine)? |
|
Sure - I'll give it a whirl in a new PR and see what happens... |
| @@ -184,15 +184,14 @@ public virtual void AddScheme(AuthenticationScheme scheme) | |||
| /// <param name="name">The name of the authenticationScheme being removed.</param> | |||
| public virtual void RemoveScheme(string name) | |||
| { | |||
| if (!_schemes.ContainsKey(name)) | |||
| if (!_schemes.TryGetValue(name, out var scheme)) | |||
There was a problem hiding this comment.
There is also GetSchemeAsync which can be updated, but that'll probably show up in the other PR.
Edit: Looks like it did show up in the other PR 😄
There was a problem hiding this comment.
Should this one be changed given the out value is discarded?
Or:
| if (!_schemes.TryGetValue(name, out var scheme)) | |
| if (!_schemes.TryGetValue(name, out var _)) |
There was a problem hiding this comment.
Looks like #44799 found that and 4 others. Rather than churn this PR, shall I just change this and address those with a rebase once this is merged?
There was a problem hiding this comment.
Whatever's convenient for you.
|
Thanks @martincostello! Looking forward to the next PR 😃 |
Use TryGetValue
Use
TryGetValue()instead ofContainsKey()and the indexer.Description
While looking through the Identity code to work out why something I'm doing wasn't working, I spotted a case where
TryGetValue()wasn't being used to access a dictionary value. I did a quick search and found a couple more, but I didn't go through the whole repo so there might be more lurking.Enabling the new CA1854 rule would surface more of this and help stop new ones being added.