[WIP] policy: refactor of policy options and matching #1192
Conversation
Problem: the current matching functions for config-provided policies don't take advantage of some C++20 features, are a bit confusing, and have no unit tests. Refactor and add unit testing.
| matcher = std::make_shared<greater_interval_first_t> (); | ||
| } else if (policy == VAR_AWARE) { | ||
| } else if (policy == "VAR_AWARE") { |
There was a problem hiding this comment.
I think it's usually discouraged to have lots of string literals lying around. The more times you type out the string, the greater the chance that you'll mistype it once and it can sometimes be pretty annoying to debug. If you refer to the string using a variable (like VAR_AWARE here), at least the compiler will tell you if you mistype the name. So in this switchyard I would probably go for something like if policy == policies["firstnodex"] rather than if policy == "FIRST_NODEX_MATCH".
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1192 +/- ##
========================================
- Coverage 73.9% 73.9% -0.1%
========================================
Files 102 103 +1
Lines 14565 14576 +11
========================================
+ Hits 10766 10773 +7
- Misses 3799 3803 +4 |
|
Oh, oops, I had forgotten this was open as a PR. Ignore that push, that was just to sync across machines. |
Problem: the current matching functions for config-provided policies don't take advantage of some C++20 features, are a bit confusing, and have no unit tests.
Refactor and add unit testing.