codecov-commenter
commented
11 months ago Codecov Report
Attention: 8 lines in your changes are missing coverage. Please review.
Comparison is base (
130d81e) 77.07% compared to head (29ddf3f) 85.64%.
| Files | Patch % | Lines |
|---|---|---|
| src/NexusMods.Paths/Trees/Traits/IHaveKey.cs | 97.34% | 0 Missing and 7 partials :warning: |
| src/NexusMods.Paths/Trees/Traits/IHaveParent.cs | 99.56% | 0 Missing and 1 partial :warning: |
Additional details and impacted files
```diff @@ Coverage Diff @@ ## main #30 +/- ## ========================================== + Coverage 77.07% 85.64% +8.57% ========================================== Files 35 44 +9 Lines 1823 2939 +1116 Branches 311 507 +196 ========================================== + Hits 1405 2517 +1112 + Misses 361 357 -4 - Partials 57 65 +8 ``` | [Flag](https://app.codecov.io/gh/Nexus-Mods/NexusMods.Paths/pull/30/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nexus-Mods) | Coverage Δ | | |---|---|---| | [Linux](https://app.codecov.io/gh/Nexus-Mods/NexusMods.Paths/pull/30/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nexus-Mods) | `85.23% <99.28%> (+8.82%)` | :arrow_up: | | [Windows](https://app.codecov.io/gh/Nexus-Mods/NexusMods.Paths/pull/30/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nexus-Mods) | `85.23% <99.28%> (+8.82%)` | :arrow_up: | | [macOS](https://app.codecov.io/gh/Nexus-Mods/NexusMods.Paths/pull/30/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nexus-Mods) | `85.16% <99.28%> (+8.86%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nexus-Mods#carryforward-flags-in-the-pull-request-comment) to find out more.:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Resolves #26
Details
Read the original issue and/or documentation (Introduction) for details.
The system should be fairly well documented.
Test Coverage
100% line coverage across 1358 code statements, and 158 tests total.
Misc. Notes
Source Generation
This tree system is prime candidate for source generation (as mentioned during end of last week in Retrospective).
It might be possible to dedupe some of the
Array&ObservableCollectioncode (where there are no API differences). That would be done by wrapping the collections in a struct constrained toIEnumerable<T>and/or other interfaces as needed.Problem is, C# compiler can 'lower' usage of
IEnumerable(such asforeach) into more performantforloop, when it encounters types like array etc. I don't know if that would be preserved if using this constraint. This is currently unproven.Alternatively, T4 templates may be used, since the API differences between
Array&ObservableCollectionare not that huge. I actually think that may be a decent approach to this. Might put up an issue for this if we ever need to add more APIs here.Performance
Although no new benchmarks join the fray, the code is guaranteed zero overhead (source:
trust me bro).All trait methods use zero cost abstractions (or none at all, if necessary) to get the job done. They're as fast as if they were manually written into the class by a human; and faster than anything in our existing
FileTree.There are some things that might be worth benchmarking, e.g. whether
GetChildrenRecursiveUnsafeis fasterbreadth firstordepth first. For now I haven't had a chance to find out.General Outlook
Fairly happy with what you can achieve using this system.
Although the implementation is a bit funny, with respects to de-virtualization and all, I think the zero cost trait based system emulates Rust quite well.
If you're reviewing this, bear in mind the
ObservableCollection&Arraystuff is almost entirely duplicated, so if you see one, you can skim/skip the other.