Most frequent element in a C++ List, STL?

Question

I have a program, where I enter some numbers in a C++ STL list. I want to find the most frequent element of the list. My question is what am I doing the wrong way, since the program does not work as expected?

// List.cpp : Defines the entry point for the console application.
//

#include <iostream>
#include <algorithm>
#include <list>
#include <vector>
using namespace std;
char number;
int counter;
list <char> l;
vector <int> m;

list <char>::iterator START;
void countRepetition();
int main()
{
    do {
        number = getchar();
        if (number != '0') {
            l.push_back(number);
        }
    } while (number != '0');

    /*for (START = l.begin(); START != l.end(); START++) {
        m.push_back(countRepetition(*START));
    }

    for (int i = 0; i < m.size(); i++) {
        cout << m[i] << endl;
    }
    */
     countRepetition();
    auto x = max_element(m.begin(), m.end());
    cout << *x << endl;
    return 0;
}
void countRepetition() {
    for (auto i = l.begin(); i != l.end(); i++) {
        for (auto j = l.begin(); j != l.end(); j++) {
            if (*i == *j) {
                counter++;
            }
            m.push_back(counter);
        }
    }
}

I would advise you to create variables at the first instance where they are used not in the beginning, because it makes your code harder to read, which causes the problem here. — M. Denninger, Apr 17 '18 at 12:08
It would be really easy to refactor the code to not make use of global variables and it would be a lot clearer in the process. — Borgleader, Apr 17 '18 at 12:12
And you are returning the maximum _count_ not the most frequent element. — Thinkeye, Apr 17 '18 at 12:13
Hmm, question to clarify the answer of [question](https://stackoverflow.com/questions/49613269/finding-most-common-element-in-a-list-c-stl). I think loop will continue until you read some [good c++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) — HMD, Apr 17 '18 at 12:15
@Ron - none of the listed headers are to blame. There is actually a needed header that is not included. — Peter, Apr 17 '18 at 12:15
Once you've fixed your code, submit it to https://codereview.stackexchange.com, they'll help you write more idiomatic, modern C++ code — papagaga, Apr 17 '18 at 12:16
Why have you asked this question again? What was wrong with [this onr](https://stackoverflow.com/questions/49613269/finding-most-common-element-in-a-list-c-stl)? — NathanOliver, Apr 17 '18 at 12:54

M. Denninger · Answer 1 · 2018-04-17T12:31:24.247

I think what you want is something like:

void countRepetition() {
    for (auto i = l.begin(); i != l.end(); i++) {
        int counter = 0;
        for (auto j = l.begin(); j != l.end(); j++) {
            if (*i == *j) {
                counter++;
            }
        }
        m.push_back(counter);
    }
}

I would further advise to make m and l parameters to the function: void countRepetition(const list<char>& l, vector<int>& m)

void countRepetition(const list<char>& l, vector<int>& m) {
    for (auto i = l.begin(); i != l.end(); i++) {
        int counter = 0;
        for (auto j = l.begin(); j != l.end(); j++) {
            if (*i == *j) {
                counter++;
            }
        }
        m.push_back(counter);
    }
}

EDIT: (Thanks to papagaga)

With a map this can be solved even nicer:

void countRepetition(const list<char>& l, map<char, int>& m) {
    for(const auto& element : l){
        ++m[element];
    }
}

you could also mention: `std::map repetitions; for (auto i : l) ++repetitions[i];` — papagaga, Apr 17 '18 at 12:18

Alexey Usachov · Answer 2 · 2018-04-17T13:07:49.657

Also you can use simple algorithm for_each + map container to calculate unique appearances.

#include<iostream>
#include<list>
#include<map>
#include<algorithm> 
using namespace std;

int main() 
{
    list <char> Lst;
    map<char,int16_t> MapCounter;

    //Fill list by data
    char Arr[] {1,1,2,3,4,5,6,7,4,2,1};
    back_insert_iterator<decltype(Lst)> InsListIt(Lst);
    copy(&Arr[0], &Arr[12], InsListIt);

    //Calc unique appearance of elements. Store results in MapCounter of all unique elements apperances
    for_each(Lst.begin(), Lst.end(), [&MapCounter](int val){ MapCounter[val]++; });

    //Calc element that appears max frequeantly times in list
    char MaxElement = 0;
    int16_t MaxRepeat = 0;
    for_each(MapCounter.begin(), MapCounter.end(), [&MaxElement, &MaxRepeat](pair<char, int16_t> el)
    {
        if ( MaxRepeat < el.second )
        {
            MaxElement = el.first;
            MaxRepeat = el.second;
        }
    });

    return 0; 
}

Most frequent element in a C++ List, STL?

2 Answers2