finding the min and max element in vector using std::min_element, std::max_element

Question

I am new to C++, and I am trying to find the minimum and maximum elements of a std::vector, but both std::min_element() and std::max_element() are not working together. The given output is only the minimum value. In the output, only the minimum value is printed twice, instead of minimum first then maximum.

#include <iostream> 
#include <algorithm> 
#include <vector>
using namespace std;

bool comp1_(int a, int b) 
{ 
    return a > b; 
}

bool comp2(int a, int b) 
{ 
    return a < b; 
}

int main() 
{ 
    vector<int> myvector;
    vector<int>::iterator i1;
    vector<int>::iterator i2;
    int n, num;
    cin >> n;
    for(int i = 0; i < n; i++){
        cin >> num;
        myvector.push_back(num);
    }
    i2 = std::min_element(myvector.begin(), myvector.end(), comp2);
    cout << *i2 << " ";
    i1 = std::max_element(myvector.begin(), myvector.end(), comp1_); 
    cout << *i1; 
    return 0; 
}

Your comparer for max_element is returning the wrong value. From cppreference: "... returns true if the first argument is less than the second." Your comparer is doing the opposite. — Remy Lebeau
– Remy Lebeau, Commented Sep 16, 2018 at 19:34

score 5 · Accepted Answer · 2018-09-16 20:51:59Z

Your problem is simply that you use max_element incorrectly. It expects a comparison which returns true if the first argument is less than the second. You will need to use comp2_ in both cases. So in your program it should read

i1 = std::max_element(myvector.begin(), myvector.end(), comp2);

The best practice to achieve what you want is using minmax_element like Jesper mentioned

#include <iostream> 
#include <algorithm> 
#include <vector>


bool compLess(int a, int b)
{
    return (a < b);
}

int main()
{
    using namespace std;
    vector<int> myvector;

    int n, num;
    cin >> n;
    for (int i = 0; i < n; i++) {
        cin >> num;
        myvector.push_back(num);
    }

    auto minmax = std::minmax_element(myvector.begin(), myvector.end(), compLess);
    cout << "min: " << *minmax.first << "\tmax:" << *minmax.second << "\n";
    return 0;
}

With this you do not have to iterate twice through the array.

Hint Do not use using namespace std; in global namespace. I would consider that to be bad practice.

john · Accepted Answer · 2018-09-16 19:35:20Z

0

i1 = std::max_element(myvector.begin(), myvector.end(), comp1_);

is wrong

i1 = std::max_element(myvector.begin(), myvector.end(), comp2);

would work.

By using comp1_ with max_element you are effectively asking for the minimum element because max_element finds the element x for which comp1_(x, y) is not true for any y.

You've effectively done a double negative, you've switched to asking for the maximum element but by also reversing the comparison function you've switched to asking for the minimum again.

What you should do when using min_element and max_element is pass a comparison function that means 'less than', then these functions will do what they say.

You might now realise that

i2 = std::min_element(myvector.begin(), myvector.end(), comp1_);

would get you the maximum element.

edited Sep 16, 2018 at 19:35

answered Sep 16, 2018 at 19:26

john

88.3k5 gold badges61 silver badges95 bronze badges

1 Comment

Jesper Juhl Over a year ago

You may want to mention std::minmax_element since OP is looking for both min and max.

saccharios · Accepted Answer · 2018-09-16 19:34:32Z

0

You need to use comp2 as well for the call of std::max_element. The comparison operator is for both min_element and max_element the same.

Alternatively you could also use i1 = std::min_element(myvector.begin(), myvector.end(), comp1_);

Check out also https://en.cppreference.com/w/cpp/algorithm/max_element.

answered Sep 16, 2018 at 19:34

saccharios

11 bronze badge

Collectives™ on Stack Overflow

finding the min and max element in vector using std::min_element, std::max_element

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related