How is my code?

[closed account 3R5fjE8b]

I'm a beginner C++ programmer, and attempting to step out of my comfort zone, this is my first time using map. This program is designed to parse string input and calculate the number of appearances of each letter. How is this? Thanks for your time!

#include <iostream>
#include <ctype.h>
#include <map>
using std::map;
using std::string;
using std::cin;
using std::cout;
using std::endl;

class Parser {
public:
    Parser();
    void get_input();
    void calculate_words();
    void calculate_letters();
    void print_results();
    void clear_results();

private:
    map<int, int> frequency;
    int numWords;
    string input;
};

Parser::Parser()
{
    /*initialize keys in map to the int values of capital alphabetical characters ( 65 - 90 )*/
    enum Alphabet { A = 65, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z};
    for ( unsigned int index = A; index <= Z; index++ )
    {
        frequency[index] = 0;
    }
    numWords = 0;
}

void Parser::get_input()
{
    /*get input, convert to all uppercase*/
    cout << "Please enter a sentence.\n" << endl;
    cout <<  "> ";
    getline(cin, input);
    cout << "You entered : " << input << endl;
    for ( unsigned int index = 0; index < input.size(); index++ )
    {
        input[index] = toupper(input[index]);
    }
}

void Parser::calculate_words()
{
    /*Cycle through string, when a non-alphabetical character is encountered, and the previous index was alphabetical, add to word total.*/
    for (unsigned int index = 0; index <= input.size(); index++)
    {
        if ( !isalpha(input[index]) && isalpha(input[index - 1]) )
        {
            numWords++;
        }
    }
    cout << "Number of words - " << numWords << endl;
}

void Parser::calculate_letters()
{
    /*converts char at current index into int, finds the appropriate key in map, increment.*/
    for ( unsigned int index = 0; index <= input.size(); index++ )
    {
        frequency[input[index]]++;
    }
}

void Parser::print_results()
{
    enum Alphabet_cast { A = 65, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z };
    char Alphabet_char[] { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z' };
    unsigned int alphabet_counter = 0;
    for ( unsigned int index = A; index <= Z; index++ )
    {
        if ( frequency[index] != 0 )
        {
            cout << "Character - " << Alphabet_char[alphabet_counter] << " Frequency - " <<  frequency[index] << endl;
        }
        alphabet_counter++;
    }
}

void Parser::clear_results()
{
        enum Alphabet { A = 65, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z};
        for ( unsigned int index = A; index <= Z; index++ )
        {
            frequency[index] = 0;
        }
    numWords = 0;
}

int main()
{
    bool endProgram = false;
    while ( !endProgram )
    {
        Parser parser;
        parser.get_input();
        parser.calculate_words();
        parser.calculate_letters();
        parser.print_results();
    }
    return 0;
}

[LB]

You are overcomplicating things.

I would strongly recommend not using enums for this - they over complicate the solution and just lead to a lot of typing.

I have the same to say about your class - the task you set out to complete is a simple one and only becomes more complicated and messy when you try to solve it with a class.

Try first writing the program that counts occurrences of letters - if you need anything other than the main function or if your code spans more than 30 lines, you're overcomplicating. Then, you should be able to just add a few lines to be able to count the number of words using your existing solution (hint: what separates words?).

Even bigger hints:
http://www.cplusplus.com/reference/cctype/isalpha/
http://www.cplusplus.com/reference/cctype/isspace/

Just so there's no confusion: this sort of problem is a good use for std::map.

[closed account 3R5fjE8b]

How could you shorten it? I'm very logical in my thinking and tend to do it step by step, which may make additional code. I used the isalpha function, and also tested for empty space.

[anup30]

input:

max 4.2 kg

output:

number of words -2
.....

input:

a big tree

output:

number of words -3
.....

[LB]

How could you shorten it? Get rid of that Parser class. It's not really a class, just a collection of functions that aren't really functions either. You just call them each once in sequence, and they take no parameters and return no values. There's no point to either the class or the functions - they add needless complication.

You're trying to tighten screws with a chainsaw. Think simpler.

[closed account 3R5fjE8b]

So just perhaps a struct, with some functions?

[closed account 3R5fjE8b]

I noticed that as well anup30, I need to fix that!

[LB]

Nope, no struct either. Try this template:

#include <iostream>
#include <map>
#include <cctype>

int main()
{
    std::map<char, unsigned> occurrences;
    //... write code here...
}