-5

I am trying to reverse the order of the words contained in a string using Stack in C++, but I am getting spaces at the beginning of the string in my final output.

Please help me find the error and solve it.

Here is my code:

#include <iostream>
#include <stack>
#include <string>
using namespace std;

int main()
{
    int number;
    cin >> number; // number of strings to reverse

    for(int i = 0; i <= number; i++)
    {
        string str;
        getline(cin, str); // input

        std::stack<std::string> s; //stack
        string str1 = "";

        for(int i = 0; i <= str.length(); i++)
        {
            if(str[i] == ' ' || str[i] == '\0')
            { // add the word whenever it encounters a space
                s.push(str1); //push the string
                str1 = "";
            }
            else
                str1 += str[i];
        }

        while(!s.empty())
        { // output
            cout << s.top(); // simple displaying without space
            cout << " ";
            s.pop();
        }
    }

    cout << '\n';
}

Input:

2
Reverse this String
OOP and DS

Output:

         String this Reverse 
DS and OOP 

Expected Output:

String this Reverse
DS and OOP
user4581301
  • 33,082
  • 7
  • 33
  • 54
Tanvi
  • 1
  • 1
  • 2

2 Answers2

1

You are close to the solution, but you have hit a behaviour that is not intuitive. Your problem is that when you cin the number, the enter key that you press at the end is also taken by the following getline, leading to an empty string. So getline gives you a total of 3 strings, which clearly isn't what you want: you want to have 2, the first one being "Reverse this string" and the second one "OOP and DS", nothing more. What you did was working with 3, because there was a "ghost" one at the beginning. You used a "strange" exit condition for the loop over i (normally i<number is used, but you used i<=number), which makes me think you must have noticed that with 2 iterations you were skipping "OOP and DS", and you tried to solve the problem, but you found a workaround instead of applying the correct solution.

As others have said, you can add cin.ignore(10000,'\n'); to solve the problem. This way, getline will capture the right amount of lines (that is, 2). Then you have to restore the right exit condition, that is,

for(int i = 0; i < number; i++)

The other loop also looks suspicious:

for(int i = 0; i <= str.length(); i++)

Here you did the same thing (you used <= instead of <) but it turns out it's ok, because you are reading one character after the end of the string, which will give you the terminating character '\0' which you are checking for, so it's fine.

Apart from this I have just moved the cout << endl; inside the for loop, after every reverted line has been printed. Oh, and I have changed the variable of the inner loop from i to j: it's better to avoid reusing the same names (i is already used by the outer loop), because they can be confusing (though in this case they work well).

This is the final result (there are also a couple of couts that I added for debugging and that I left, though commented):

#include <iostream>
#include <stack>
#include <string>
using namespace std;

int main()
{
    int number;
    cin >> number; // number of strings to reverse
    cin.ignore(10000,'\n');

    for(int i = 0; i < number; i++)
    {
        string str;
        getline(cin, str); // input
        //cout << "Debug: i = " << i << ", string = " << str << endl;

        std::stack<std::string> s; //stack
        string str1 = "";

        for(int j = 0; j <= str.length(); j++)
        {
            if(str[j] == ' ' || str[j] == '\0')
            { // add the word whenever it encounters a space
                //cout << "Pushing '" << str1 << "'" << endl;
                s.push(str1); //push the string
                str1 = "";
            }
            else
                str1 += str[j];
        }

        while(!s.empty())
        { // output
            cout << s.top(); // simple displaying without space
            cout << " ";
            s.pop();
        }
        cout << endl;
    }

}
1

Fabio Turati has the right of the cause and better wording than I did, so at this point I'm just adding this as a more elegant solution and to demonstrate a few solutions to other pitfalls the OP will run up against.

#include <iostream>
#include <stack>
#include <string>
#include <limits> // needed by std::numeric_limits<std::streamsize>::max()
#include <sstream> // needed by std::stringstream

// my hatred of using namespace std; knows no bounds so I've replaced it, only
// pulling in a few conveniences
using std::cin;
using std::cout;
using std::endl;

int main()
{
    int number; 
    cout << "Please input the number of strings to reverse" << endl;
    // give the user a prompt so they know what to do
    while (! (cin >> number) || number <= 0) // will loop until the user gives
                                             // us a good number and the number
                                             // is not 0
    {
        cin.clear(); // clear the error flag
        cout << "try again, wise guy." << endl; // mock user's stupidity
        cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        // blow away whatever else user typed in up to the EOL or a few 
        // gajillion entered characters, whichever comes first.
    }
    // OK so we finally got a usable number from the user.
    cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    // throw away anything else the user typed because we don't want to reverse it.
    for(unsigned int i = 0; i < number; i++) // reduced the range of the for loop
    {
        std::string str;
        cout << "Please input a strings to reverse" << endl; 
        // let the user know what's expected
        std::getline(cin, str);
        std::stringstream line(str); // allows us to easily split words just like cin
        std::stack<std::string> wordstack; // more descriptive name. Helps in debugging
        std::string word; //more descriptive name

        while (line >> word) // don't need a for loop here anymore
        { //See? Parsing made easy!
            wordstack.push(word); //push the string
        }

        while(!wordstack.empty())
        { // output
            cout << wordstack.top() << " ";
            wordstack.pop();
        }
        cout << endl; // \n does not force the output to the console. endl does,
                      // but  as a result it is much slower. This happens once
                      // per string and makes the output look prettier, so it's
                      // worth it.
    }
}
user4581301
  • 33,082
  • 7
  • 33
  • 54
  • I think using `unsigned int` for `number` doesn't do what you want. If I enter -1 it will be silently converted to `std::numeric_limits::max()`, which is normally 4294967295. I'd rather use a signed int and verify that it's a positive number. Then, I think your while loop to validate the first input should rather be like this: `while ( !(cin >> number) || number <= 0)`. Other than this, I like your approach. Your "parsing made easy" really works well! – Fabio says Reinstate Monica Aug 27 '15 at 23:12
  • @FabioTurati The second point is me being stupid and will be fixed momentarily. The first point... Crom. You have got to be kidding me. Just tested. You are not kidding me. Wow. One less reason to use it. Easy fix here, though. – user4581301 Aug 28 '15 at 00:26
  • Great. You know, when I saw that `cout << "try again, wise guy." << endl; // mock user's stupidity` I immediately fell in love with your answer :-), and tried to run it with wrong input, just to see that. Which is actually why I discovered those problems. I didn't upvote because of this, but now I have no reserve about it. By the way, I think you've done a great job at giving more meaningful names and commenting. Keep it up! – Fabio says Reinstate Monica Aug 28 '15 at 00:48