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

Process string vector and change each string to uppercase

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

Problem

The question below came from C++ primer 5th edition.


Read a sequence of words from cin and store the values a
vector. After you’ve read all the words, process the vector and change
each word to uppercase. Print the transformed elements, eight words to a
line.

I was just wondering if there is any way to improve my solution.

int main(){

    vector stringVector;
    string s;
    while(cin >> s){
        stringVector.push_back(s);
    }
    for(string &s : stringVector){
        for(char &c : s){
            c = toupper(c);
        }
    }
    for (int i = 0; i != stringVector.size(); i++){
        if (i != 0 && i % 8 == 0){
            cout << endl;
        }
        cout << stringVector[i] << " ";
    }

    return 0;
}

Solution

I see some things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. Know when to use it and when not to (as when writing include headers).

Consider using standard algorithms

There is nothing wrong with your transformation of the words to uppercase, but it may be useful to consider an alternative using a standard algorithm. In particular std::transform may be useful here:

for(string &s : stringVector){
    std::transform(s.begin(), s.end(), s.begin(), 
        [](char c){ return std::toupper(c); });
}


Use a counter rather than the % operator

It's not really necessary to use the % operator here. It may not make any difference for this particular application, but on many processors, the % operator requires more CPU cycles (and therefore time) while counting down is usually very fast.

Minimize the number of iterations through the vector

The code currently makes three passes through the data. First, it gets the input, next it transforms to upper case, and finally it emits the opper case versions of the input words. These could all three be done in a single pass, although doing so doesn't quite match the given instructions. The advantage, however, is that now a std::vector isn't even needed any longer. Written that way the program might look like this:

#include 
#include 
#include 
#include 

int main()
{
    constexpr unsigned WORDS_PER_LINE = 8;
    std::string s;
    unsigned wordcount = WORDS_PER_LINE;
    while(std::cin >> s){
        std::transform(s.begin(), s.end(), s.begin(), 
            [](int c){ return std::toupper(c); });
        std::cout << s << ' ';
        if (--wordcount == 0) {
            std::cout << '\n';
            wordcount = WORDS_PER_LINE;
        }
    }
    std::cout << '\n';
}


Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.

Code Snippets

for(string &s : stringVector){
    std::transform(s.begin(), s.end(), s.begin(), 
        [](char c){ return std::toupper(c); });
}
#include <iostream>
#include <string>
#include <algorithm>
#include <ctype>

int main()
{
    constexpr unsigned WORDS_PER_LINE = 8;
    std::string s;
    unsigned wordcount = WORDS_PER_LINE;
    while(std::cin >> s){
        std::transform(s.begin(), s.end(), s.begin(), 
            [](int c){ return std::toupper(c); });
        std::cout << s << ' ';
        if (--wordcount == 0) {
            std::cout << '\n';
            wordcount = WORDS_PER_LINE;
        }
    }
    std::cout << '\n';
}

Context

StackExchange Code Review Q#119584, answer score: 3

Revisions (0)

No revisions yet.