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

feat: use IEnumerable Dictionary constructor #755

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

TimothyMakkison
Copy link
Collaborator

@TimothyMakkison TimothyMakkison commented Sep 15, 2023

Use IEnumerable Dictionary constructor

Description

Use the IEnumerable constructor if key and value mapping is synthetic.

// New
return new global::System.Collections.Generic.Enumerable.Dictionary<string, long>(source);

// Old
var target = new global::System.Collections.Generic.Dictionary<string, long>(source.Count);
foreach (var item in source)
{
    target[item.Key] = item.Value;
}
return target;

See #689

Checklist

  • The existing code style is followed
  • The commit message follows our guidelines
  • Performed a self-review of my code
  • Hard-to-understand areas of my code are commented
  • The documentation is updated (as applicable)
  • Unit tests are added/updated
  • Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

@TimothyMakkison TimothyMakkison force-pushed the to_dictionary branch 2 times, most recently from 3ba19c4 to 5bf76b5 Compare September 15, 2023 10:46
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #755 (2a3296c) into main (8f7528a) will decrease coverage by 0.01%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##             main     #755      +/-   ##
==========================================
- Coverage   91.31%   91.30%   -0.01%     
==========================================
  Files         201      201              
  Lines        6633     6651      +18     
  Branches      821      825       +4     
==========================================
+ Hits         6057     6073      +16     
- Misses        387      388       +1     
- Partials      189      190       +1     
Files Changed Coverage Δ
...iptors/MappingBuilders/DictionaryMappingBuilder.cs 90.55% <88.88%> (-0.28%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@latonz latonz 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 for this PR 😊

@TimothyMakkison TimothyMakkison force-pushed the to_dictionary branch 2 times, most recently from e9e8e2b to fcb1126 Compare September 15, 2023 12:58
@TimothyMakkison
Copy link
Collaborator Author

TimothyMakkison commented Sep 15, 2023

On second thoughts, should I close this PR and use the Dictionary<TKey, TValue>(IEnumerable<KeyValuePair<TKey, TValue>) constructor instead. This extension is just a wrapper for it, plus the constructor is available on all of .Net Core excluding .Net Standard 2.0.

I was planning on later refactoring this when I add a version detector class for #732 and to speed up checks for versioned methods

@latonz
Copy link
Contributor

latonz commented Sep 15, 2023

IMO if it is just a wrapper for the ctor we should use the ctor directly.

@TimothyMakkison TimothyMakkison force-pushed the to_dictionary branch 2 times, most recently from 004b76c to a9c7a6c Compare September 16, 2023 10:02
@TimothyMakkison TimothyMakkison changed the title feat: use .Net 8 ToDictionary overload feat: use IEnumerable Dictionary constructor Sep 16, 2023
@TimothyMakkison TimothyMakkison force-pushed the to_dictionary branch 3 times, most recently from 0ab9fe9 to b35aa7f Compare September 19, 2023 18:38
@latonz latonz merged commit 1bc7bdd into riok:main Sep 20, 2023
17 of 19 checks passed
@github-actions
Copy link

🎉 This PR is included in version 3.3.0-next.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants