HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Storing words from an input stream into a vector

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
streamintowordsinputfromstoringvector

Problem

I'm extremely new to C++ and am doing the exercises on the book Accelerated C++. Here is one of the exercises:


4-5. Write a function that reads words from an input stream and stores
them in a vector. Use that function both to write programs that count
the number of words in the input, and to count how many times each
word occurred.

How can I make this program better by only using vectors?

#include 
#include 
#include 
#include
#include 
using std::cout; using std::cin; using std::vector; using std::sort; using std::string; using std::endl; using std::domain_error; using std::cerr;

//it said "count the number of words in the input, and to count how many times each word occurred". I cannot think of any return value if a function do two things, so I used void.
void run(const vector& v) {
    if (v.size() == 0) throw domain_error("Nothing to count");
    cout ::size_type i = 1; i  words;
    string word;
    while (cin >> word) {
        words.push_back(word);
    }
    sort(words.begin(), words.end());
    try {
        run(words);
    } catch (domain_error e) {
        std::cerr << e.what() << endl;
    }
    return 0;
}

Solution

-
Minor inconsistency here:

#include


Add a space in between them, like all the others:

#include 


-
Just get rid of this:

using std::cout; using std::cin; using std::vector; using std::sort; using std::string; using std::endl; using std::domain_error; using std::cerr;


Even if you put them onto separate lines, it'll still be lengthy. Just put std:: where necessary and try to keep it consistent.

-
Exception objects usually should not be passed by value:

catch (domain_error e)


They should be passed by const&:

catch (domain_error const& e)


-
Consider separating this into additional lines:

if (v.size() == 0) throw domain_error("Nothing to count");


While it's okay to put short statements on a single line, it may be a little harder to read and maintain longer lines, also with the lack of curly braces.

In addition, just call empty() instead of comparing size() to 0.

if (v.empty())
{
    throw domain_error("Nothing to count");
}


-
Instead of getting the first element via []:

string unique = v[0];


use front():

string unique = v.front();


I'd also like to add a compliment:

Thank you for using std::size_type instead of int. I hardly see that done by others. It's good to see that you're paying attention to return types and compiler mismatch warnings.

Code Snippets

#include<string>
#include <string>
using std::cout; using std::cin; using std::vector; using std::sort; using std::string; using std::endl; using std::domain_error; using std::cerr;
catch (domain_error e)
catch (domain_error const& e)

Context

StackExchange Code Review Q#55244, answer score: 8

Revisions (0)

No revisions yet.