STL template function to return a pair

Question

I'm trying to write a function that returns a pair of values from an STL container.

template <typename T>
std::pair<typename T::value_type,typename T::value_type> getMinMax(T &container) {

    auto min = *(container.begin());
    auto max = *(container.begin());

    for (auto it : container) {
        if (min > (*it) ) {
            min = (*it);
        }
        if (max < (*it) ) {
            max = (*it);
        }
    }
    return std::make_pair(min, max);
};

int main() {
    std::vector<int> c{1, 2, 3, 4, 5};
    auto p = getMinMax(c);
    std::cout << "min: " << p.first << " max: " << p.second << "\n";
}

I'm getting an error:

error: indirection requires pointer operand ('int' invalid)
        if (min > (*it) ) {

I don't know how to deal with that.

Besides that error, is there a better way to implement the desired behavior?

Answer 1

for range returns element, not iterator. So your loop should be something like:

for (const auto& e : container) {
    if (min > e) {
        min = e;
    }
    if (max < e) {
        max = e;
    }
}

Answer 2

For starters the function can have undefined behavior in case when the container is empty because there can be an attempt of dereferencing the iterator of an empty container.

In loops like this

for (auto it : container) {
    if (min > (*it) ) {
        min = (*it);
    }

there is incorrectly used dereferencing.

You could use the standard algorithm std::minmax_element. However it does not do the same as your code. It returns the first minimum element and the last maximum element. So you should rewrite the algorithm std::minmax_element such a way that ir would return the first minimum element (the iterator pointing to the first minimum element) and the first maximum element (the iterator pointing to the first maximum element).

The function can be defined for example the following way

#include <iostream>
#include <utility>
#include <vector>
#include <iterator>

template <typename T>
auto getMinMax( T &container ) 
    -> std::pair<decltype( container.begin() ), decltype( container.begin() )> 
{
    auto min = container.begin();
    auto max = container.begin();

    if ( !container.empty() )
    {
        for ( auto it = container.begin(); ++it != container.end();  )
        {
            if ( *it < *min ) min = it;
            else if ( *max < *it ) max = it;
        }
    }

    return { min, max };
}

int main() 
{
    std::vector<int> v = { 5, 2, 3, 7, 1, 4, 9, 8, 6 };

    auto minmax = getMinMax( v );

    std::cout << "Minimum = " << *minmax.first 
              << " at position " << std::distance( v.begin(), minmax.first )
              << " and maximum = " << *minmax.second
              << " at position " << std::distance( v.begin(), minmax.second )
              << std::endl;

    return 0;
}

The program output is

Minimum = 1 at position 4 and maximum = 9 at position 6

Answer 3

template <typename T>
std::pair<typename T::value_type, typename T::value_type> getMinMax(T &container) {

    auto min = *(container.begin());
    auto max = *(container.begin());

    for (const auto& element : container) { /* ERROR WAS HERE, FOR RANGE LOOPS RETURN AN ELEMENT */
        if (min > element) {
            min = element;
        }
        if (max < element) {
            max = element;
        }
    }
    return std::make_pair(min, max);
};

Hey! This should work, you were setting min and max to a dereferenced element, which of course, isn't what we want. :)

Also, you can get undefined behavior with this, for example, if the container was empty. Perhaps you should add some checks that check for that.