Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance: Inline variable in CA1854 (IDictionary.TryGetValue) #7071

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Enhance: Inline variable in CA1854 (IDictionary.TryGetValue) #7071

merged 4 commits into from
Feb 23, 2024

Conversation

Poker-sang
Copy link
Contributor

When simplifying code with TryGetValue, we can inline the out variable.

Before:

if (data.TryGetValue(key, out string value))
{
    var a = value;
}

After:

if (data.TryGetValue(key, out string a))
{
}

@Poker-sang Poker-sang requested a review from a team as a code owner December 7, 2023 14:49
Copy link

codecov bot commented Dec 7, 2023

Codecov Report

Attention: Patch coverage is 92.50000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 96.45%. Comparing base (4195460) to head (f192345).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7071   +/-   ##
=======================================
  Coverage   96.45%   96.45%           
=======================================
  Files        1422     1422           
  Lines      340592   340657   +65     
  Branches    11230    11243   +13     
=======================================
+ Hits       328522   328588   +66     
+ Misses       9233     9228    -5     
- Partials     2837     2841    +4     

Copy link
Member

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Poker-sang, the changes looks good, could you please update the VB fixer too? Now this only work on C# but not in VB.

@Poker-sang
Copy link
Contributor Author

@buyaa-n Thank you for the review. I've never written VB, but I tried writing VB fixer anyway, so I'm not sure if I took all the cases into account. Please let me know if there are any problems.

@buyaa-n
Copy link
Member

buyaa-n commented Feb 23, 2024

I've never written VB, but I tried writing VB fixer anyway, so I'm not sure if I took all the cases into account. Please let me know if there are any problems.

Thanks, most contributors including myself are same, no previous VB experience, https://converter.telerik.com/ have been helpful for converting code and tests in VB, though looks you don't need it now. Changes looks good

…rmance/BasicPreferDictionaryTryMethodsOverContainsKeyGuardFixer.vb
Copy link
Member

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@buyaa-n buyaa-n enabled auto-merge (squash) February 23, 2024 01:26
@buyaa-n buyaa-n merged commit bc447c9 into dotnet:main Feb 23, 2024
11 checks passed
@Poker-sang
Copy link
Contributor Author

Thanks, most contributors including myself are same, no previous VB experience, https://converter.telerik.com/ have been helpful for converting code and tests in VB, though looks you don't need it now. Changes looks good

Thanks. I use https://sharplab.io/ to see the C#/VB syntax tree. The site you recommended looks good too, I'll try it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants