4
\$\begingroup\$

This is the follow-up question for A Summation Function For Various Type Arbitrary Nested Iterable Implementation in C++. Besides the summation case, I am trying to implement a Max function which can find the maximum value in various type (comparable) arbitrary nested iterable things as the further step. The recursive technique is also used here in order to iterate all elements. Some concepts as the constraints for template are as below.

template<typename T>
concept Summable = requires(T x) { x + x; };

template<typename T>
concept Iterable = requires(T x)
{
    x.begin();      // must have `x.begin()` 
    x.end();        // and `x.end()` 
};

The key implement part about this Max function.

template<typename T> requires Summable<T>
static T Max(T inputNumber);        //  Deal with the base case like "Max(static_cast<int>(1))"

template<class T> requires Iterable<T>
static auto Max(const T& numbers);  //  Deal with the iterable case like "Max(std::vector<long double>{ 1, 1, 1 })"


template<class T> requires Summable<T>
static inline T Max(T inputNumber)
{
    return inputNumber;
}

template<class T>  requires Iterable<T>
static inline auto Max(const T& numbers)
{
    typedef typename std::iterator_traits<typename T::iterator>::value_type
        value_type;

    decltype(Max(std::declval<value_type &&>())) maxValue{};

    maxValue = static_cast<decltype(maxValue)>(Max(numbers.at(0)));
    for (auto& element : numbers)
    {
        if (maxValue < Max(element))
        {
            maxValue = Max(element);
        }
    }
    return maxValue;
}

The std::vector test case to this Max function.

std::vector<int> v{ 1, 2, 3, -1, -2, -3 };
int largest_element = *std::max_element(v.begin(), v.end());
std::cout << largest_element << std::endl;
std::cout << Max(v) << std::endl;

The value of largest_element and the result of Max(v) are the same.

The std::array test case to this Max function.

std::array<long double, 10> testArray;
for (size_t i = 0; i < 10; i++)
{
    testArray[i] = i;
}
std::cout << Max(testArray) << std::endl;

The more complex (std::vector<std::vector<long double>>) case:

std::vector<long double> testVector1;
testVector1.push_back(1);
testVector1.push_back(20);
testVector1.push_back(-100);

std::vector<long double> testVector2;
testVector2.push_back(10);
testVector2.push_back(90);
testVector2.push_back(-30);

std::vector<std::vector<long double>> testVector3;
testVector3.push_back(testVector1);
testVector3.push_back(testVector2);
std::cout << Max(testVector3) << std::endl;

All suggestions are welcome.

The summary information:

  • Which question it is a follow-up to? A Summation Function For Various Type Arbitrary Nested Iterable Implementation in C++

  • What changes has been made in the code since last question?

    The previous question focus on the summation operation and the main idea in this question is trying to achieve the maximum operation.

  • Why a new review is being asked for?

    In my opinion about this code, there are some problem existed. Although this Max function seems works well, there is a little weird to the usage of Summable concept here. Is it a good idea to replace this Summable into a new concept Comparable? If so, Is there any better idea or suggestion about the design of Comparable? I have ever comes up an idea to the concept Comparable like below.

    template<typename T>
    concept Comparable = requires(T x) { x > x; };
    

    However, the operator ">" has been defined in std::vector until C++20. This operator > compares the contents lexicographically. This causes the error "ambiguous call to overloaded function".

    Moreover, is it a good idea to simplify the initial value assignment maxValue = static_cast<decltype(maxValue)>(Max(numbers.at(0))); into maxValue = Max(numbers.at(0)) (remove the static_cast syntax)?

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Using the right concept

Indeed, Summable is the wrong concept to use if you want to check if the value type is such that you can calculate the maximum of two values. Have a look at the documentation of std::max(). In particular, it requires that the type is LessThanComparable. So you could create a concept to test that. There is already a concept std::totally_ordered, which is a bit more strict than necessary.

Simplify Max()

You do an elaborate dance to get the right type for maxValue, but why not just use auto? Also, use std::max() to calculate the maximum for you:

template<class T>  requires Iterable<T>
static inline auto Max(const T& numbers)
{
    auto maxValue = Max(numbers.at(0));

    for (auto& element : numbers)
    {
        maxValue = std::max(maxValue, Max(element));
    }

    return maxValue;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.