-11

For example I have vector {'a','a','b','b','c'} and I want to get the most letters which is a and b but this code the output is a;

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

int getMostFrequentElement(std::vector<char> &arr)
{
    if (arr.empty())
        return -1;

    std::sort(arr.begin(), arr.end());

    auto last_int = arr.front();
    auto most_freq_int = arr.front();
    int max_freq = 0, current_freq = 0;

    for (const auto &i : arr) {
        if (i == last_int)
            ++current_freq;
        else {
            if (current_freq > max_freq) {
                max_freq = current_freq;
                most_freq_int = last_int;
            }

            last_int = i;
            current_freq = 1;
        }
    }

    if (current_freq > max_freq) {
        max_freq = current_freq;
        most_freq_int = last_int;
    }

    return most_freq_int;
}

int main(){
    std::vector<char> arr = {'a','a','b','b','c'};

    char ret = getMostFrequentElement(arr);
  
    std::cout << "Most frequent element = " << ret;

    
}

May I know why my output becomes a instead a and b?

input vector arr{'a','a','b','b','c'} expected output is a and b

but my output is a

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
anony
  • 42
  • 7
  • Please edit your post with the results of your debugging session. Which statement is causing the issue? What are the values of key variables? What are their expected values? – Thomas Matthews Sep 30 '21 at 20:14
  • 6
    [Team up with this asker](https://stackoverflow.com/q/69398131/4581301). You have a lot in common, right down to the same code. – user4581301 Sep 30 '21 at 20:15
  • 1
    TL;DR version: A function that returns one value can't be expected to return two values without modification. – user4581301 Sep 30 '21 at 20:16
  • 1
    _"May I know why my output becomes a instead a and b?"_ Did someone else write this code? It is difficult to imagine that you wrote this code, expecting `char ret` to have the value `"a and b"`. – Drew Dormann Sep 30 '21 at 20:28
  • @anony The function returns only the first most frequent character as an integer in a sorted vector. – Vlad from Moscow Sep 30 '21 at 20:31
  • 3
    Cheating on a homework assignment might get you through the class, but do you think if you try to get a job, people won't figure it out. Cheating isn't going to teach you to program. – Joseph Larson Sep 30 '21 at 20:31
  • @JosephLarson As far as I know all who does their test assignments at first tries to find a solution in the internet.:) – Vlad from Moscow Sep 30 '21 at 20:34
  • A function with a return type of `char` returns **one** character only. If you need to return multiple characters, use `std::vector` as the return type and modify your function accordingly. – Wais Kamal Sep 30 '21 at 20:38
  • 1
    It would seem that this code was lifted from [here](https://www.delftstack.com/howto/cpp/find-most-frequent-element-in-an-array-cpp/), retrofitted somewhat to work with `char` instead of `int`. – Drew Dormann Sep 30 '21 at 20:41

2 Answers2

0

Your function returns only the first most frequent character as an integer in a sorted vector.

For starters the implementation of the function is not good. The function shall not sort the passed by reference vector. It is the owner of the vector decides whether to sort the vector before calling the function. The function shall not modify the passed to it vector.

If you want that the function would return all most frequent characters in a vector then you need to change the function essentially.

For example the function can look the following way as it is shown in the demonstrative program below.

#include <iostream>
#include <vector>
#include <map>
#include <iterator>
#include <algorithm>

std::vector<char> getMostFrequentElement( const std::vector<char> &v )
{
    std::vector<char> result;
    
    std::map<char, size_t> m;
    
    
    for ( const auto &c : v ) ++m[c];
    
    auto it = std::max_element( std::begin( m ), std::end( m ), 
                                []( const auto &p1, const auto &p2 )
                                {
                                    return p1.second < p2.second;
                                } );

    if ( it != std::end( m ) )
    {
        for ( const auto &p : m )
        {
            if ( p.second == it->second ) result.push_back( p.first );
        }
    }
    
    return result;
}

int main() 
{
    std::vector<char> v = { 'a', 'a', 'b', 'b', 'c' };
    
    auto result = getMostFrequentElement( v );
    
    for ( const auto &c : result ) std::cout << c << ' ';
    std::cout << '\n';
    
    return 0;
}

The program output is

a b
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

The answer from Vlad is good and should be accepted.

I would like to show an additional, more "mordern" C++ solution.

The Function body is rather compact and consists only of 3 lines of code. It will count all occurences of char and sort it in decreasing order regarding the occurence.

So, the caller of this function can show all kind of information. In the example below, I show all topmost elements.

But all kind of other evaluations may be shown.

Please see:

#include <iostream>
#include <vector>
#include <utility>
#include <algorithm>
#include <set>
#include <iterator>
#include <unordered_map>

// Writing some aliases to prevent later typing work and make the code a little bit more readable. ---------------------
using DataType = char;
using CounterType = unsigned int;
using Pair = std::pair<DataType, CounterType>;
using Counter = std::unordered_map<DataType, CounterType>;
using Data = std::vector<DataType>;
struct Comp { bool operator ()(const Pair& p1, const Pair& p2) const { return (p1.second == p2.second) ? p1.first<p2.first : p1.second>p2.second; } };
using CountedAndSorted = std::multiset<Pair, Comp>;
// ----------------------------------------------------------------------------------------------------------------------


CountedAndSorted getMostFrequentElement(Data& data) {
    
    // Count
    Counter counter{};
    for (const char c : data) counter[c]++;
    
    // Return counted and sorted result
    return {counter.begin(), counter.end()};
}


// ------------------------
// Test/Driver code
int main() {
    
    // Test Data
    Data d = { 'a', 'a', 'b', 'b', 'c' };
    
    // Calculate result
    auto result = getMostFrequentElement(d);
    
    // Show output
    for (const auto& [c, count] : result) if (count == result.begin()->second) std::cout << c << ' ';
}
A M
  • 14,694
  • 5
  • 19
  • 44