0

So, I have the following code: 

#include <iostream>
#include <unordered_map>
#include <utility>
#include <map>
using namespace std;

struct cmp 
{
    bool operator() (const pair<int,int>&a, const pair<int,int>&b)
    {
        if (a.first == b.first)
            return a.second<b.second;
        else
            return a.first > b.first;
    }
};
int main (void)
{
    int i=0,n,foo;
    cin>>n;
    map <int, pair<int,int>, cmp > mymap;
    while (i != n)
    {
        //auto it = mymap.begin();
        cin>>foo;
        if (mymap.find(foo) == mymap.end())
        {
            mymap[foo].make_pair(1,i);
        }
        else
        {
            mymap[foo].first++; 
        }
        i++;
    }
    auto it = mymap.begin();
    while (it != mymap.end())
    {
        cout<<it->first<<"\t"<<it->second.first<<"\t"<<it->second.second;
        cout<<"\n";
    }
    return 0;

}

What this code does is basically takes input elements and sorts them by frequency (in decreasing order). And if frequency of two elements are same, then it outputs that which appeared first in the input list. 

For ex: I/P: 2 5 2 8 5 6 8 8 
O/P: 8 8 8 2 2 5 5 6

I have doubts in two places. Firstly, in the comparator function which I wrote, it seems to be correct but it gives an error on compilation which is, 

no matching function for call to object of type 'const cmp'
        {return static_cast<const _Hash&>(*this)(__x);}

Also, the loop that I am using for printing the output, is that correct, especially, it->second.first?

John Lui
  • 1,434
  • 3
  • 23
  • 37

2 Answers2

1

From cppreference.com, std::unordered_map is defined as

template<

    class Key,
    class T,
    class Hash = std::hash<Key>,
    class KeyEqual = std::equal_to<Key>,
    class Allocator = std::allocator< std::pair<const Key, T> >
> class unordered_map;

As this container is unordered, it doesn't need comparator to sort elements.

So, define your (unordered) map as

    unordered_map <int, pair<int,int> > mymap;

i.e. without the cmp.

Also, don't forget toincrement the iterator in your last while-loop, i.e. add a ++it, or else ...

Your example should compile now:

$ ./mwe 
4
3
2
1
5
5   1   3
1   1   2
2   1   1
3   1   0

UPDATE

With reference to your comment, if you change the type of mymap from std::unordered_map to std::map, bare in mind that elements will be sorted by their keys. And that shouldn't be changed. If you want to sort the elements by values, consider this alternative:

Use a std::vector, add a struct that includes both, the above key and value, and update your cmp accordingly, e.g.

#include <algorithm>
#include <vector>
#include <tuple>

typedef std::tuple< int /* key */, int /* pair::first */, int /* pair::second */ > Element;
typedef std::vector< Element > ElementList;

template <int N>
bool cmp( const Element & a, const Element & b )
{
    return std::get< N >( a ) < std::get< N >( b );
}

int main()
{
    ElementList list;

    // Just dummy data, but you get the idea.
    list.push_back( std::make_tuple( 1, 2, 3 ) );
    list.push_back( std::make_tuple( 2, 3, 4 ) );
    list.push_back( std::make_tuple( 3, 4, 5 ) );

    // Sort by "pair::first", which is at element index 1 of 
    // the tuple, cf. cmp<1> below. Change this template parameter 
    // to the index value of the tuple element that should be used to 
    // sort your data.
    std::sort( list.begin(), list.end(), cmp<1> );

    return 0;
}
nils
  • 2,424
  • 1
  • 17
  • 31
  • Do you really need to create a custom comparator to compare 2 ints? – Slava Jul 01 '15 at 21:51
  • @Slava Thaha, so eager to write an answer that I didn't notice the obvious. Thanks for the gentle hint. – nils Jul 01 '15 at 21:58
  • Can you please elaborate on how exactly does your comparator work? I have a very trivial doubt here, sometimes we enclose the bool operator inside a struct (as I did in my code) and sometimes, we write the direct bool operator as you did, what's the difference between the two and what to use when? Secondly, why do you pass `1` in the `cmp`? – John Lui Jul 01 '15 at 22:00
  • You used what's called a Functor, my example uses a function pointer. For details, this [SO question](http://stackoverflow.com/questions/6451866/why-use-functors-over-functions) could be start. The <1> is a template parameter; I added a comment to the example above. If you like the answer feel free to mark it as accepted. – nils Jul 03 '15 at 06:30
0

maps are sorted by their keys, not their values. Consider using a set instead.

rlbond
  • 65,341
  • 56
  • 178
  • 228