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

Group RGI interface and implementation functions separately. #33

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

tanaya-mankad
Copy link
Contributor

Instead of grouping functions in the implementation file in a publicA-implA, publicB-implB sequence, group all functions from the same class together. This allows us the flexibility of adding implementations that might be used in multiple public interfaces, for example. Secondary grouping is to match the order functions are declared in their respective header files (public first, etc.)

This PR also addresses a minor change in the code checking grid-axis for sortedness, to maintain a comparison operator supporting strict weak ordering for the std::is_sorted function.

@codecov-commenter
Copy link

Codecov Report

Merging #33 (42fd4ac) into main (9bab8ed) will increase coverage by 3.32%.
Report is 72 commits behind head on main.
The diff coverage is 85.88%.

@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   82.56%   85.88%   +3.32%     
==========================================
  Files           5        7       +2     
  Lines         453      517      +64     
==========================================
+ Hits          374      444      +70     
+ Misses         79       73       -6     
Flag Coverage Δ
integration 85.88% <85.88%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
include/btwxt/grid-axis.h 0.00% <0.00%> (ø)
include/btwxt/grid-point-data.h 0.00% <0.00%> (ø)
include/btwxt/logging.h 0.00% <0.00%> (ø)
src/regular-grid-interpolator-implementation.h 4.55% <4.55%> (ø)
src/grid-axis.cpp 85.33% <85.33%> (-8.00%) ⬇️
src/regular-grid-interpolator-implementation.cpp 91.11% <91.11%> (ø)
src/regular-grid-interpolator.cpp 97.87% <97.87%> (+11.13%) ⬆️

@nealkruis
Copy link
Member

That's pretty phenomenal coverage with those tests!

@tanaya-mankad
Copy link
Contributor Author

Huh, that's interesting about the tests. Did that happen as a result of refactoring the impl class into its own file?
I have reverted the is_sorted change until we have a chance to understand it better. I'll post it as an Issue so we don't forget about it but for now I think we ought to keep this PR clean with just reorganization.

@nealkruis nealkruis merged commit 5363cdd into main Aug 10, 2023
6 checks passed
@nealkruis nealkruis deleted the group-implementations-by-class branch August 10, 2023 23:43
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.

4 participants