Letter frequency by using map container

Question

I am attempting to define a class named Frequency by using map container as a data structure. The class is intended to read characters from the keyboard and count the frequency of each character that has been input.

This shows the UML diagram for the class Frequency. The function printFreqTable should print the frequency of each input character as shown in the sample output:

Enter your string without spaces: textEPF
Char   Frequency
E      *
P      *
F      *
t      **
e      *
x      *

However, my implementation does not produce any outputs, and exists with code -1073741819. I was also wondering if there is a more efficient method of implementation that uses a map container.

#include <iostream>
using namespace std;
#include <map>
#include <string>
#include <vector>

class Frequency {
public:
    Frequency();
    void insertStringtoMap();
    void printFreqTable();
    void readString();
private:
    typedef map <char, string, less <char > > mid;
    mid CMap;
    string s;
};

Frequency::Frequency()
{
    readString();
}

void Frequency::readString()
{
    cout << "Enter your string without spaces: ";
    getline(cin, s);
    cout << endl;
}

void Frequency::insertStringtoMap()
{
    int si = s.size();
    char str[50];
    strcpy_s(str, s.c_str());
    string freq = 0;

    for (char ch = 65; ch <= 122; ch++) {
        int c = 0;
        for (int i = 0; i < si; i++)
        {
            if (ch == str[i])
            {
                c++;
            }
        }
        if (c > 0)
        {
            for (int star = 0; star < c; star++)
            {
                freq = freq + "*";

            }       
            CMap.insert(mid::value_type(ch, freq));
        }
    }
}

void Frequency::printFreqTable()
{
    cout << "Char\tFrequency";
    for (auto iter = CMap.cbegin(); iter != CMap.end(); ++iter)
        {
            cout << iter->first << "\t"
            << iter->second << "\n";
        }
}

int main()
{
    Frequency f;
    f.insertStringtoMap();
    f.printFreqTable();
}

score 0 · Answer 1 · answered Nov 05 '18 at 12:27

I've changed to counting the occurrences of each character instead of building the graphical representation directly. When the counting is done I've used std::string's capability to create a string with n copies of a certain character to create the diagram.

#include <iostream>
#include <map>
#include <string>
#include <vector>

class Frequency {
public:
    Frequency();
    void insertStringtoMap();
    void printFreqTable();
    void readString();
private:
    typedef std::map<char, size_t> mid;
    mid CMap;
    std::string s;
};

Frequency::Frequency() :
    CMap(),
    s()
{
    readString();
}

void Frequency::readString()
{
    std::cout << "Enter your string without spaces: ";
    std::getline(std::cin, s);
    std::cout << std::endl;
}

void Frequency::insertStringtoMap() {
    for(auto ch : s) {
        ++CMap[ch];
    }
}

void Frequency::printFreqTable()
{
    std::cout << "Char\tFrequency\n";
    for (const auto& e : CMap)
    {
        std::cout << e.first << "\t" << std::string(e.second, '*') << "\n";
    }
}

int main()
{
    Frequency f;
    f.insertStringtoMap();
    f.printFreqTable();
}

P.W · Answer 2 · 2018-11-05T12:46:44.960

There are three issues with your code.
First:

string freq = 0; //Assigning an int literal to a string?

It should simply be:

string freq;

Second:
This should be inside the first for loop next to int c = 0; so that you reset the count of *s for each character.

for (char ch = 65; ch <= 122; ch++) {
    int c = 0;
    string freq;

Third:
This is just to make the output look better, add a new line in this statement like this:

cout << "Char\tFrequency\n";

Output:

Enter your string without spaces: asdsadfkdfgag

Char    Frequency
a       ***
d       ***
f       **
g       **
k       *
s       **

As a good practice, stop using using namespace std. See this question and its answers: Why is "using namespace std" considered bad practice?

score 0 · Answer 3 · answered Nov 05 '18 at 12:43

I think you're getting confused by trying to do too many things at once.

Split your problem into 2 parts: read the string and store each occurrence in the map, and print the resulting map in a pretty format.

Now you can parse the string into the map easily, simply loop over each character and add an entry to the map - which can be anything, so a std::map<char, int> will store an int (ie of occurrences) against each key entry, which can be the character itself

do this by resolving the map entry and incrementing the value, eg mymap[letter]++ or mymap.find(letter) += 1

then work on the formatting, by iterating through the map, printing the same number of stars as the int value.

(in pseudocode:)

std::map<char,int>::iterator i = mymap.begin()
while (i != mymap.end()) {
  cout i.first << "*" (i.second) times << endl;
  i++;
}

Don't bother storing the * string in the map, just keep a note of the number of occurrences and use that to generate the star string when needed. That's much more efficient, and you can also change your formatting very easily.

notes: use a typedef for your map which makes things look and read much easier. Std::string has a constructor that easily lets you generate a number of stars in a string.

Letter frequency by using map container

3 Answers3