CODEWITHSUNDEEP
c++sortingvector
Divya garg
Divya garg

Reputation: 35

why is sorting not working in vector of pairs

I am trying to sort a vector of pairs but I am not able to understand what is error in this code.

#include <bits/stdc++.h>

bool sortbysec(const pair<int,int> &a, const pair<int,int> &b) { 
    return (a.second < b.second); 
} 

using namespace std;

int main() {
    // ios_base::sync_with_stdio(false);
    // cin.tie(NULL);

    long int t, i;
    cin >> t;
    vector<pair<int, int>> vect;
    pair<int, int> tmp;
    for (i = 0; i < t; i++) {
        cin >> tmp.first;
        vect.push_back(tmp);
    }
    sort(vect.begin(), vect.end(), sortbysec); 

    return 0;
}

Someone please help me to understand what is wrong in this piece of code.

Upvotes: 0

Views: 735

Answers (4)

brc-dd
brc-dd

Reputation: 13074

More modern (and compact) version of your code (C++14):

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

int main() {
    long int t;
    std::cin >> t;
    std::vector<std::pair<int, int>> v(t);
    for (auto &i : v)
        std::cin >> i.first >> i.second;
    std::sort(v.begin(), v.end(), [](const auto &a, const auto &b) {
        return a.second < b.second;
    });
    for (auto &i : v)
        std::cout << i.first << ' ' << i.second << '\n';
    return 0;
}

Upvotes: 1

Deepak Tatyaji Ahire
Deepak Tatyaji Ahire

Reputation: 5321

I agree with @WhozCraig. Your custom comparator is comparing only the .second element of each pair, which you never set to anything.

Try taking input for the .second value.

Have a look at the following implementation:

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

bool sortbysec(const std::pair<int,int> &a, const std::pair<int,int> &b) { 
    return (a.second < b.second); 
} 

int main() {

    int t, i;
    std::cin >> t;

    std::vector<std::pair<int, int>> vect;

    std::pair<int, int> tmp;

    for (i = 0; i < t; i++) {
        std::cin >> tmp.first;
        std::cin >> tmp.second;
        vect.push_back(tmp);
    }

    sort(vect.begin(), vect.end(), sortbysec); 

    for (i = 0; i < t; i++) {
        std::cout<<vect[i].first<<" "<<vect[i].second<<std::endl;
    }

    return 0;
}

Input:

5
1 5
2 4
3 3
4 2
5 1

Output:

5 1
4 2
3 3
2 4
1 5

PS: Have a look on Why should I not #include bits/stdc++.h?

Also, Why is “using namespace std;” considered bad practice?

Upvotes: 1

Coral Kashri
Coral Kashri

Reputation: 3506

You are sorting only by the pair .second value, and input only the .first value. To solve this issue you can:

1. Change your sort function to sort by pair .first value:

bool sortbyfirst(const pair<int,int> &a, const pair<int,int> &b) { 
    return (a.first < b.first); 
}

2. Input your to pair .second value:

for (i = 0; i < t; i++) {
    cin >> tmp.first;
    cin >> tmp.second; // Add this line
    vect.push_back(tmp);
}

3. {{Combine}} Input both .first & .second, and sort by both:

bool sort_by_first_and_sec(const pair<int,int> &a, const pair<int,int> &b) {
    return a.first <= b.second && a.second < b.second; 
}

Upvotes: 1

Anil Gupta
Anil Gupta

Reputation: 145

Firstly, i would like to point out your errors
1. Always write using namespace std after header files, since you are writing below your custom comparator so the compiler could not able to resolve what pair is, since it belongs to std namespace.
2. Since you are sorting your pair by second value you should take second value in input as well. 

#include <bits/stdc++.h>
using namespace std;

bool sortbysec(const pair<int,int> &a, const pair<int,int> &b) 
{ 
    return (a.second < b.second); 
} 

int main(){
    // ios_base::sync_with_stdio(false);
    // cin.tie(NULL);

    long int t, i;
    cin >> t;
    vector<pair<int, int> > vect;
    pair<int, int> tmp;

    for(i=0; i<t; i++){
        cin>>tmp.first >> tmp.second;
        vect.push_back(tmp);
    }
    cout << "Before Sorting" << "\n";
    for (int i = 0; i < t; i ++) {
        cout <<vect[i].first << " " << vect[i].second << "\n";
    }
    sort(vect.begin(), vect.end(), sortbysec);
    cout << "after Sorting" << "\n";
    for (int i = 0; i < t; i ++) {
        cout <<vect[i].first << " " << vect[i].second << "\n";
    }

    return 0;
}

I have refactored your code, hope this helped.
Input
4
2 3
4 1
2 6
3 2
Output
Before Sorting
2 3
4 1
2 6
3 2
after Sorting
4 1
3 2
2 3
2 6

Upvotes: 2

Related Questions