Fix `max_value` and `min_value`
This merge request adds a test for rangeutilities.hh and corrects max_value and min_value.
Merge request reports
Activity
@christi: Could you take a look as you wrote the original implementation?
The test looks good to me. I simplified the fix of
is_range(see the discussion in #58 (closed)) and includedIteratorRangein the static checks to make sure that at least one of our own classes fulfills theis_rangeconcept.Once the test succeeds we can merge.
Mentioned in commit 3e67db3d
Mentioned in commit c45cdffe
Mentioned in merge request !186 (merged)
Mentioned in commit 12f8f5b7
30 31 typename std::enable_if<is_range<T>::value, int>::type = 0> 31 32 typename T::value_type 32 33 max_value(const T & v) { 33 using std::max; 34 typename T::value_type m; 35 for (const auto & e : v) 36 m = max(e,m); 37 return m; 34 using std::max_element; You are aware that there is a subtle change in semantics and a bug here? Think of an empty range. Don't you dereference the end iterator now instead of returning the default initialized value_type (which also seems like a bug). Now you have a logic bug and a memory bug.
Edited by Markus BlattWell, that just means calling
max_valueon an empty range is not allowed. (I mentioned this in fd080e1f)The code looked like it intentionally avoids conditionals, so I tried to follow that (note that
any_truedoesn't short-circuit).
-
@markus.blatt there is no stl equivalent. The stl version works an iterator pair, but what we need is something that works on the container. This is the only way we can use this for things like std::array, FieldVector and SIMD-data types. The implementation (as it checks for is_range) even works for scalar data types.
mentioned in commit d2d0f3ae
