map operations(find most occurence element)

Question

here is code

#include <iostream>
#include <map>
using namespace std;
int main(){
map<int ,int>a;
    map<int,int>::iterator it;
    int b[]={2,4,3,5,2,6,6,3,6,4};
     for (int i=0;i<(sizeof(b)/sizeof(b[0]));i++){
         ++a[b[i]];
     }
    // for (it=a.begin();it!=a.end();it++){
        // cout<<(*it).first<<" =>"<<(*it).second<<"\n";
     //}
     int max=a.begin()->second;
     for (it=a.begin();it!=a.end();it++){
         if ((*it).second>max){
             max=(*it).second;
         }
     }
     for (it!=a.begin();it!=a.end();it++){
         if ((*it).second==max){
             cout<<(*it).first<<"\n";
         }
     }




      return 0;
}

what i am trying is following according to number of occurence of each keys i want it print element which occurs most often in this case 6 but it does not show me result what is mistake?

One tip is not to reuse variables for different purposes in a function. That would have found this problem early. If the type name is sooo big you find it hinders readability, use a `typedef`. `typedef map::iterator MapIt; for (MapIt it = ...)` — Stephen, Jul 21 '10 at 16:49

score 4 · Accepted Answer · answered Jul 21 '10 at 16:46

You have a typo in your second loop. This:

for (it!=a.begin();it!=a.end();it++){

Should be this:

for (it=a.begin();it!=a.end();it++){

By the way, (*it).first can be more idiomatically written as it->first. The arrow operator (->) is a combination of the dereference (*) and member access (.) operators.

score 3 · Answer 2 · answered Jul 21 '10 at 16:48

3

Your second loop should begin with it=a.begin(), not it!=a.begin(). But why don't you just capture the number that's occurring most frequently when you capture max, and get rid of the second loop altogether?

answered Jul 21 '10 at 16:48

Owen S.

7,665
1
28
44

score 1 · Answer 3 · answered Jul 21 '10 at 17:35

Given what you're doing, you might consider using a Boost bimap instead of a plain map. With it, your code would work out something like this:

#include <boost/bimap.hpp>
#include <boost/bimap/list_of.hpp>
#include <iostream>

int main() {    
    int b[]={2,4,3,5,2,6,6,3,6,4};

    boost::bimap<int, boost::bimaps::list_of<unsigned> > a;

    for (int i=0; i<elements(b); i++)
        ++a.left[b[i]];

    std::cout << a.right.rbegin()->second;
}

The one bit of ugliness here is that it depends on what would be the value type to initialize itself properly in the default ctor, which int doesn't, so we have to change it from list_of<unsigned> to list_of<uint_proxy>, with uint_proxy defined something like this:

class uint_proxy {
    unsigned value;
public:
    uint_proxy() : value(0) {}
    uint_proxy& operator++() { ++value; return *this; }
    unsigned operator++(int) { return value++; }
    operator unsigned() const { return value; }
};

In this case, the proxy type adds a bit more length than we'd like, but the main stream of the code is still pretty simple by comparison, and will generally be considerably more efficient -- in particular, it avoids the linear scan to find the key that had the highest count. For the test case that's probably irrelevant, but with a large amount of data is likely to be more significant.

map operations(find most occurence element)

3 Answers3

Linked