Process string vector and change each string to uppercase

Question

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

Answer 1

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

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.

• Jerry Coffin: A few minor notes: you should convert to unsigned char before passing to toupper. Although it doesn't change the externally visible behavior, storing the strings in a vector is a specific requirement of the exercise.
• Thor: @Edward Thank you so much for the detailed explanation! so great to have people like you helping out beginners!! Really appreciate you time and help!