# Exercise from Accelerated C++

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;
}


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) ...
• On the correct line but you can simplify. – Martin York Jun 7 '12 at 4:26
• At first glance, I don't quite understand how this works, but I will research how to use a map, thank you! – newToProgramming Jun 7 '12 at 11:38
• Assuming a new compiler and no requirement to print the results in order, you might also consider std::unordered_map instead of std::map. Also, though it makes no difference here, absent a reason to do otherwise, you generally want to use pre-increment instead of post-increment (++tMap[word];). – Jerry Coffin Jun 7 '12 at 16:09
• @Jerry Coffin Yes I am using a new compiler, Thank you for your help! – newToProgramming Jun 7 '12 at 21:44

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)

• I thought about using iterators, but that accesses elements at random right? – newToProgramming Jun 7 '12 at 11:39
• @newToProgramming: No. For vectors. begin() is index 0 and ++ moved through the vector in order. For Map/Set the order is the sorted order of the container. – Martin York Jun 7 '12 at 15:21
• I am having trouble implementing std::copy from your code snippet, but I will figure it out, thank you for all of the corrections, I clearly have a lot to learn about using c++ correctly. Thanks. – newToProgramming Jun 7 '12 at 22:33
• ok, i figured it out, the second argument in std::copy is initialized with the istream_iterator default constructor which points to end-of-file, so it should be istream_iterator<std::string>() or just omit the paranthesis entirely. Is that correct? – newToProgramming Jun 7 '12 at 22:51
• Fixed the std::copy() above. You need the braces in this context. – Martin York Jun 8 '12 at 3:37

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)

• You are right of course, but I think I will switch to using an iterator. – newToProgramming Jun 7 '12 at 21:51
• You can just use auto. – Archie Gertsman Jun 28 '16 at 9:39