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
Multiple enumeration #4682
Multiple enumeration #4682
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4682 +/- ##
==========================================
- Coverage 27.57% 27.55% -0.02%
==========================================
Files 508 508
Lines 40989 40992 +3
Branches 5965 5977 +12
==========================================
- Hits 11301 11296 -5
- Misses 29179 29185 +6
- Partials 509 511 +2
Continue to review full report at Codecov.
|
If implementation of IEnumerable<T> also implements ICollection<T> or ICollection, then the corresponding Count property can be used directly, avoiding enumeration.
5b71372
to
8511b13
Compare
Removed most occurrences of IList. Remaining ones cannot easily be removed.
The values were cached, but in a presentation that didn't suit their usage. This change stores them at the type they need to be. Nest methods.
Went through and addressed most of the remaining |
IList<GitRevision> items = new List<GitRevision> { _headRevision, baseCommit }; | ||
var items = new List<GitRevision> { _headRevision, baseCommit }; | ||
|
||
// TODO this can never be true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this in separate commit?
@@ -69,6 +65,53 @@ public void Initialize(bool remote, string[] containRevisons) | |||
{ | |||
Branches.Text = null; | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return; here for clarity?
Changes proposed in this pull request:
IReadOnlyList<T>
across all ofGitModule
and places that then must change in response to thatAs mentioned elsewhere, this PR explores broader use of
IReadOnlyList<T>
. Currently quite a lot of data is passed around as the modifiableIList<T>
which makes sense given GE has been around for longer than the comparatively newIReadOnlyList<T>
. HoweverIList<T>
leaves you wondering, is it safe to modify this collection?Before this PR there were 140 instances of
"IList<"
betweenGitCommands
andGitUI
projects. After, there are 56.Interested in thoughts on this direction before going after the last instances.
What did I do to test the code and ensure quality:
Has been tested on: