STL sort function in C++ wrt strings

Question

So I've been trying to sort a string based on the frequency of its characters. However the online judge I've been using shows me the error
Line 17: invalid use of non-static member function 'bool olution::helper(char, char)'
Why is the call to my function wrong? I have used the sort() function before, but not to strings. Is my helper() function incorrect?

class Solution {
public:
unordered_map<char,int> freq;

bool helper(char c1,char c2){
    if(freq[c1]>freq[c2]) return false;
    else return true;
}
string frequencySort(string s) {

    for(char c:s)
    {
        freq[c]++;
    }

    sort(s.begin(),s.end(),helper);

    return s;
}
};

Get a [few good C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282) to read, and learn C++ properly from the beginning. Then you will also learn what's wrong with the code you now have. — Some programmer dude, Jul 04 '18 at 15:13
Different letters could have the same frequency. Maybe do a normal `sort(begin, end)` followed by a `stable_sort(begin, end, frequency_test)` according to frequency? — Galik, Jul 04 '18 at 15:20
`if (a > b) return false; else return true;` --> `return a <= b;`. — molbdnilo, Jul 04 '18 at 15:23
A [Compare](https://en.cppreference.com/w/cpp/named_req/Compare) predicate must return `false` if you are comparing a value to itself, yours does not. Just `return freq[c1] < freq[c2];` — Caleth, Jul 05 '18 at 09:18

Olivier Sohn · Answer 1 · 2018-07-04T16:03:43.667

1

Use a lambda to capture this:

sort(s.begin(),s.end(),[this](auto a, auto b) -> bool { return helper(a,b); });

edited Jul 04 '18 at 16:03

answered Jul 04 '18 at 15:19

Olivier Sohn

1,292
8
18

score 0 · Answer 2 · edited Jul 05 '18 at 07:25

Why is the call to my function wrong? I have used the sort() function before, but not to strings. Is my 'helper()' function incorrect?

Because helper is member function of Solution. When you do this

sort(s.begin(),s.end(),helper);

you are basically doing this

sort(s.begin(),s.end(),this->helper);

The 3rd parameter to sort needs to be a standalone function, a predicate, a functor or a lambda. It cannnot be a non-static member of a class

This code, cleaned up, works. Note the statics

class Solution {
public:
    // using thread_local so that each thread
    // has its own global variable.
    static thread_local std::unordered_map<char, int> freq;

    static bool helper(char c1, char c2) {
        return (freq[c1]<freq[c2]);
    }

    std::string frequencySort(std::string s)
    {
        freq.clear();

        for (char c : s)
            ++freq[c];

        std::sort(s.begin(), s.end(), helper);

        return s;
    }
};

// definition
std::unordered_map<char, int> Solution::freq;

score 0 · Answer 3 · answered Jul 05 '18 at 09:27

Member functions have a hidden parameter that becomes this. You need either expose the state more widely, or write a capturing lambda

Also a Compare predicate must return false if you are comparing a value to itself, yours does not.

class Solution {
public:
    string frequencySort(string s) {

        unordered_map<char,int> freq;

        for(char c:s)
        {
            freq[c]++;
        }

        sort(s.begin(),s.end(),[&freq](char lhs, char rhs){ return freq[lhs] < freq[rhs]; });

        return s;
    }
};

STL sort function in C++ wrt strings

3 Answers3

Linked

Related