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

Port and update of SortedVector #43

Merged
merged 3 commits into from
Dec 8, 2018
Merged

Port and update of SortedVector #43

merged 3 commits into from
Dec 8, 2018

Conversation

feartheducky
Copy link
Contributor

Updated SortedVector and added to Mezz_Foundation.

Updated SortedVector and added to Mezz_Foundation.
@feartheducky feartheducky self-assigned this Dec 6, 2018
@codecov
Copy link

codecov bot commented Dec 6, 2018

Codecov Report

Merging #43 into master will decrease coverage by 0.31%.
The diff coverage is 95.55%.

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
- Coverage    98.8%   98.49%   -0.32%     
==========================================
  Files           8        9       +1     
  Lines         420      465      +45     
==========================================
+ Hits          415      458      +43     
- Misses          5        7       +2

1 similar comment
@codecov
Copy link

codecov bot commented Dec 6, 2018

Codecov Report

Merging #43 into master will decrease coverage by 0.31%.
The diff coverage is 95.55%.

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
- Coverage    98.8%   98.49%   -0.32%     
==========================================
  Files           8        9       +1     
  Lines         420      465      +45     
==========================================
+ Hits          415      458      +43     
- Misses          5        7       +2

@Sqeaky
Copy link
Member

Sqeaky commented Dec 8, 2018

On line 131 in the tests, Why start with a a 5 element destination vector? The source vector has six elements. Just quirky. At first glance these all looked like the same counts, and someone reading that might think this is required for some reason.

In the tests for adding a range, the range added is sorted. Reading the code and seeing how the sorted values are interleaved make it clear that this is sorting correctly. Including a unsorted item for the vector. So again not a major issue,I wouldn't make a commit just to fix this.

I am not a huge fan of the 0 == (FindifFail - FindifEnd) because it seems more convoluted than just comparing the iterators, but seems like a way to avoid to avoid the streaming requirement of TEST_EQUAL. I opened ticket against Mezz_Test to fix this, so this is probably the best solution we have now: BlackToppStudios/Mezz_Test#47

This looks good to merge.

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.

3 participants