C++ vector prints out weird elements

Question

I'm currently in the process of learning C++ and for that I'm reading the book "C++ Primer". The book is pretty good so far and I have learned a lot however I experienced weird behaviour using a vector and I'm unsure if this is right or if it's a problem from my side.

The task is:

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."

This is my code:

#include <iostream>
#include <vector>

using namespace::std;

int main()
{
    string input;
    vector<string> svec;

    while (cin >> input)
    {
        svec.push_back(input);

        for (auto& rows : svec)
        {
            for (auto& element : rows)
            {
                element = toupper(element);
            }
        }

        int maxWordsPerLine = 0;

        for (auto word : svec)
        {
            if (maxWordsPerLine >= 8)
            {
                cout << endl;
                cout << word;
                maxWordsPerLine = 1;
            }
            else
            {
                cout << word;
                maxWordsPerLine++;
            }
        }
    }
}

I believe it does the things described in the task but when I type in:

Hello thanks for helping I dont know whats wrong with this problem lol

The output is:

HELLOHELLOTHANKSHELLOTHANKSFORHELLOTHANKSFORHELPINGHELLOTHANKSFORHELPINGIHELLOTHANKSFORHELPINGIDONTHELLOTHANKSFORHELPINGIDONTKNOWHELLOTHANKSFORHELPINGIDONTKNOWWHATSHELLOTHANKSFORHELPINGIDONTKNOWWHATS
WRONGHELLOTHANKSFORHELPINGIDONTKNOWWHATS
WRONGWITHHELLOTHANKSFORHELPINGIDONTKNOWWHATS
WRONGWITHTHISHELLOTHANKSFORHELPINGIDONTKNOWWHATS
WRONGWITHTHISPROBLEMHELLOTHANKSFORHELPINGIDONTKNOWWHATS
WRONGWITHTHISPROBLEMLOL

I hope someone can explain me why this happens and how I can avoid it in the future.

Unrelated: convert the string to uppercase before adding it to the vector. Right now every time you add an item to the vector you convert every string in the vector to uppercase, including those already converted. [Read this for more](https://www.joelonsoftware.com/2001/12/11/back-to-basics/), particularly the bit about Shlemiel the painter. — user4581301, Nov 19 '21 at 19:46
A couple issues are happening simultaneously here. First, when you print the contents of `svec`, you don't print any spaces between the words. Second, your print loop is inside your ` while(cin >> input)` loop. So every time a word gets added, you print all the words that have been added at some point up until then. — Nathan Pierson, Nov 19 '21 at 19:46
Try setting `input`'s value programmatically. Just paste the text into the code to hard-code it. If the output is normal then, then the issue is with with either cout or cin. If that's the case, you can input the text normally through cin and immediately output it without modifying it, to see if it's stored properly. If it is, then the problem is either in your algo or cin. I ran your code in godbolt.org with the text hard-coded and the output is fine (for the first iteration of `while`). — selamba, Nov 19 '21 at 19:47
Thank you all for your answers I didn't knew you need to do it before storing it in a vector. And yes I forgot the spaces, I realized it after creating the question. Thank you all again. — , Nov 19 '21 at 19:52
You don't _need_ to do it before storing it in the vector. The extra work your program is doing is unnecessary, not outright incorrect. Right now, as written, when you add word #10 to `svec`, you also re-convert words #1-9 to uppercase, which is just wasted effort. — Nathan Pierson, Nov 19 '21 at 19:54
A book that teaches you to use "using namespace std" is [not a good book](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). — n. m. could be an AI, Nov 19 '21 at 20:48

score 3 · Accepted Answer · answered Nov 19 '21 at 19:47

You need to realize there's two steps.

First step: read all the words and convert each to uppercase

Second steps: print all the words

Second step needs to be done after first step is done. However, you have a single while loop. Didn't run it, but simplest change that looks likely to work is:

string input;

vector<string> svec;

while (cin >> input)
{
    svec.push_back(input);

    for (auto& rows : svec)
    {
        for (auto& element : rows)
        {
            element = toupper(element);
        }
    }
} // extra closing bracket for the while

    int maxWordsPerLine = 0;

    for (auto word : svec)
    {
        if (maxWordsPerLine >= 8)
        {
            cout << endl;

            cout << word << " "; // extra space to separate words

            maxWordsPerLine = 1;
        }
        else
        {
            cout << word;

            maxWordsPerLine++;
        }
    }

Vlad from Moscow · Answer 2 · 2021-11-19T20:16:30.133

For starters you need to include the header <string>

#include <string>

In this while loop

while (cin >> input)
{
    svec.push_back(input);

    for (auto& rows : svec)
    {
        for (auto& element : rows)
        {
            element = toupper(element);
        }
    }
    //...

you are converting all entered words at given iteration to upper case again and again.

And outputting the vector must be removed from the while loop.

So the while loop can look the following way

while ( cin >> input )
{
    svec.push_back(input);

    for ( auto& element : svec.back() )
    {
        element = toupper(element);
    }
}

After that you may output the vector.

And in this range based for loop

for (auto word : svec)

you should not create a copy of a string stored in the vector. You should declare the variable word as having constant referenced type as

for ( const auto &word : svec)

Also the inner if statement has duplicated code that is not a good style of programming.

        if (maxWordsPerLine >= 8)
        {
            cout << endl;

            cout << word;

            maxWordsPerLine = 1;
        }
        else
        {
            cout << word;

            maxWordsPerLine++;
        }

Rewrite the if statement within the range based for loop for example the following way

        cout << word << ' ';

        if ( ++maxWordsPerLine == 8)
        {
            cout << endl;
            maxWordsPerLine = 0;
        }

C++ vector prints out weird elements

2 Answers2