Exercise from Accelerated C++

Question

I am working through the exercises in the book Accelerated C++. I wanted to get some feedback on proper use of the Standard Library, correctness, overall style, portability, etc.

/*******************************************************************************
Accelerated C++ Chapter 3

Write a program to count how many times each distinct word appears in its input
*******************************************************************************/

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

int main()
{
    std::cout << "Enter a list of words followed by end-of-file: ";

    typedef std::vector<std::string>::size_type vector_sz;
    std::vector<std::string> words;
    std::string word;
    while (std::cin >> word)
    {
        words.push_back(word);
    }

    vector_sz size = words.size();
    if(size == 0)
    {
        std::cout << "Please enter some words!";
        return -1;
    }

    std::vector<int> frequency;
    std::vector<std::string> unique_words;
    int count = 0;
    bool isUnique;

    unique_words.push_back(words[0]);

    for(unsigned i = 0; i < words.size(); ++i)
    {
        isUnique = true;

        for(unsigned j = 0; j < unique_words.size(); ++j)
        {
            if(words[i] == unique_words[j])
            {
                isUnique = false;
            }
        }
      if(isUnique)
      {
          unique_words.push_back(words[i]);
      }
    }

    for(unsigned i = 0; i < unique_words.size(); ++i)
    {
        for(unsigned j = 0; j < words.size(); ++j)
        {
            if(unique_words[i] == words[j])
            {
                count += 1;
            }
        }
        frequency.push_back(count);
        count = 0;
    }

    for(unsigned i = 0; i < unique_words.size(); ++i)
    {
        std::cout << unique_words[i] << "   " << frequency[i] << std::endl;
    }
    return 0;
}

Answer 1

You could cut your code in half if you used a map:

typedef   std::map<std::string, unsigned int> Map;

Map tMap;
while(std::cin >> word)
{
    // When accessing a map with `[]` If the value does not exist
    // it is created and initialized to zero.
    //
    // The plus plus then increments the stored value
    tMap[word]++;
}

// Iterate through map and print frequency

Couple other notes from the code you have:

• After unique_words.push_back(words[0]); you can start the i loop at index 1 instead of 0.
• When checking for uniqueness in the first pair of loops, once you determine uniqueness to be false you can stop looping for that word: while(j < unique_words.size() && isUnique) ...

Answer 2

See @sfpiano about using a map directly to count each word.

This is perfectly fine.

    std::vector<std::string> words;
    std::string word;
    while (std::cin >> word)
    {
        words.push_back(word);
    }

But if you want to learn the STL then you can replace the above with a single line:

    std::copy(std::istream_iterator<std::string>(std::cin),
              std::istream_iterator<std::string>(),
              std::back_inserter(words));

You don't need to store the value (especially since you don't re-use it)

    vector_sz size = words.size();
    if(size == 0)

Do this in a single line:

    if (words.size() == 0)

But this test is embodied in a single call.

    if (words.empty())

Here you use two vectors.

    std::vector<int> frequency;
    std::vector<std::string> unique_words;

This can be replaced by a std::map (see sfpiano) for details.

Declare as close to the use point as possible.

    bool isUnique;

This is not used outside the loop. So declare it inside the loop (and initialize at declaration.

    // So it should look like this.
    for(unsigned i = 0; i < words.size(); ++i)
    {
        bool isUnique = true;

Also you should start using iterators rather than using the index.

    for(auto loop = words.begin(); loop != words.end(); ++loop)

Answer 3

As long as you've typedef'ed std::vector<std::string>::size_type as vector_sz, use it in the code

for(vector_sz i = 0; i < words.size(); ++i)