jmalkin commented on code in PR #404:
URL: https://github.com/apache/datasketches-cpp/pull/404#discussion_r1382129001
##########
kll/include/kll_sketch.hpp:
##########
@@ -162,6 +167,7 @@ class kll_sketch {
public:
using value_type = T;
using comparator = C;
Review Comment:
I think we've been moving towards using the full name in the template and a
`using` statement to declare the single-letter version?
##########
kll/include/kll_sketch_impl.hpp:
##########
@@ -895,8 +860,8 @@ uint32_t kll_sketch<T, C,
A>::get_num_retained_above_level_zero() const {
template<typename T, typename C, typename A>
void kll_sketch<T, C, A>::check_m(uint8_t m) {
- if (m != DEFAULT_M) {
- throw std::invalid_argument("Possible corruption: M must be " +
std::to_string(DEFAULT_M)
+ if (m != kll_constants::DEFAULT_M) {
Review Comment:
Why do we bother having this as a class member if it is impossible to change
it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
